Skip to content

feat: add course assignment system with submissions and grading#25

Open
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
ayesha1145:feat/assignment-system
Open

feat: add course assignment system with submissions and grading#25
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
ayesha1145:feat/assignment-system

Conversation

@ayesha1145
Copy link
Copy Markdown
Contributor

@ayesha1145 ayesha1145 commented Mar 26, 2026

Adds a complete assignment system to the education platform, allowing hosts to create assignments and students to submit and receive grades.

Changes

schema.sql + src/worker.py DDL:

  • New assignments table: title, encrypted description, due date, max score, draft/published status, late submission flag
  • New submissions table: encrypted text response, file URL, score, encrypted feedback, grading metadata
  • 3 indexes for performance

src/worker.py — 7 new API endpoints:

  • GET /api/activities/:id/assignments — list assignments (hosts see all, students see published only)
  • POST /api/activities/:id/assignments — create assignment [host only]
  • GET /api/assignments/:id — get assignment details + student's own submission
  • PUT /api/assignments/:id — update assignment [host only]
  • DELETE /api/assignments/:id — delete assignment + submissions [host only]
  • POST /api/assignments/:id/submit — submit assignment [enrolled students, upsert]
  • GET /api/assignments/:id/submissions — list all submissions [host only]
  • POST /api/submissions/:id/grade — grade submission with score + feedback [host only]

public/course.html:

  • Assignments section below activity details
  • Host: create assignment form, view submissions, grade inline
  • Student: view published assignments, submit text response
  • Status badges (draft/published), due dates, max score display

Security

  • Assignment descriptions, text responses, and feedback encrypted with AES-256-GCM
  • Permission guards: host-only CRUD, enrollment required for submission
  • Upsert on submit — students can resubmit without duplicates
  • Score validation against max_score

This PR adds a full assignment workflow to the platform: hosts can create/publish/update/delete and grade assignments; students can view published assignments and submit/resubmit work. Changes span DB schema, backend API, and frontend UI, and include encryption, permission checks, validation, and accessibility/XSS fixes.

Database

  • New tables: assignments (id, activity_id, title, encrypted description, due_date, max_score, status draft|published, allow_late, timestamps) and submissions (id, assignment_id, student_id, encrypted text_response, file_url, status, score, encrypted feedback, graded_by, submitted_at, updated_at).
  • UNIQUE(assignment_id, student_id) enforces one submission per student.
  • Indexes added: assignments(activity_id), submissions(assignment_id), submissions(student_id).
  • Foreign keys assignments.activity_id and submissions.assignment_id use ON DELETE CASCADE; sessions.activity_id FK updated to ON DELETE CASCADE.

API

  • New authenticated endpoints with role checks:
    • GET /api/activities/:id/assignments — list (hosts see all; students see published only)
    • POST /api/activities/:id/assignments — create (host only)
    • GET /api/assignments/:id — get assignment + requesting student’s submission
    • PUT /api/assignments/:id — update (host only; preserves omitted fields vs. explicit nulls)
    • DELETE /api/assignments/:id — delete assignment and related submissions (host only)
    • POST /api/assignments/:id/submit — submit/upsert (enrolled students; rejects late when allow_late=0; resubmit clears prior grade/feedback)
    • GET /api/assignments/:id/submissions — list submissions (host only)
    • POST /api/submissions/:id/grade — grade submission with encrypted feedback (host only; score validated against assignment.max_score)
  • Server-side validations: required title, status whitelist, max_score clamping, submission must include text_response or file_url, bounded grading score.
  • AES-256-GCM encryption used for description, text_response, and feedback.
  • Submission upsert uses INSERT ... ON CONFLICT to overwrite content and null out grading metadata on resubmission.

Frontend (public/course.html)

  • Adds an Assignments section beneath activity details with host and student UIs.
    • Host: create/publish/update/delete assignments, view submissions, inline grading UI, draft/published badges.
    • Student: view published assignments, submit/resubmit text responses, see due dates and max score.
  • Replaced unsafe inline onclick handlers with data-* attributes and delegated document-level click handlers to remove XSS vectors.
  • Removed the broken recursive loadActivity wrapper (previous infinite recursion); assignments load via the activity initialization flow/DOMContentLoaded listener.
  • Added escaping helper for rendering, type="button" fixes, and ensures submissions view refreshes after grading.

Notable behavioral fixes

  • PUT distinguishes omitted keys from explicit values to avoid accidental clobbering.
  • Submissions endpoint enforces due_date when allow_late is false and rejects late submissions.
  • Resubmissions clear previous score/feedback/graded metadata so stale grades are not retained.
  • Ensures referential integrity via ON DELETE CASCADE and adds indexes for performance.

Impact

  • Requires DB migration and review of encryption key handling and auth logic.
  • Behavioral changes to due-date enforcement and automatic grade clearing on resubmission should be communicated to instructors/students.
  • Accessibility and XSS fixes improve security and usability.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 26, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds activity-scoped assignments and submissions: DB schema/tables with cascade rules and indexes, worker APIs/handlers for assignment CRUD, submit/list/grade operations with auth/validation/encryption/upsert logic, and client UI in public/course.html to list/create/submit/view/grade assignments and submissions.

Changes

Cohort / File(s) Summary
Database Schema
schema.sql
Added assignments and submissions tables with ON DELETE CASCADE, indexes, and UNIQUE(assignment_id, student_id); changed sessions.activity_id FK to REFERENCES activities(id) ON DELETE CASCADE.
Backend API / Logic
src/worker.py
Added DDL for new tables/indexes and async API endpoints: GET/POST /api/activities/:activityId/assignments, GET/PUT/DELETE /api/assignments/:id, POST /api/assignments/:id/submit, GET /api/assignments/:id/submissions, POST /api/submissions/:id/grade. Implements token verification, host checks, draft visibility, input validation/clamping, encryption/decryption of text fields, submission upsert (conflict on assignment_id,student_id), grading logic, and cascade-aware deletes.
Frontend UI
public/course.html
Added Assignments UI section and forms; client state (isHost, currentAsgnId, currentSubId); new global/window functions exposed for create/show/hide/submit/view/grade flows and helper esc2(); delegated document click handler for dynamic buttons; fetch calls and validation for new APIs; wraps window.loadActivity to load assignments after activity init.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client (Frontend)
    participant Server as Server/API
    participant DB as Database

    Client->>Server: POST /api/activities/:actId/assignments (title, description, due_date, max_score)
    Server->>Server: verify_token, check host
    Server->>Server: validate fields, clamp max_score, encrypt description
    Server->>DB: INSERT INTO assignments
    DB-->>Server: assignment row
    Server-->>Client: 201 Created + assignment
Loading
sequenceDiagram
    participant Student as Student (Frontend)
    participant Server as Server/API
    participant DB as Database

    Student->>Server: POST /api/assignments/:asgnId/submit (text_response or file_url)
    Server->>Server: verify_token
    Server->>Server: validate presence of text_response or file_url, encrypt fields
    Server->>DB: INSERT INTO submissions ON CONFLICT(assignment_id, student_id) DO UPDATE (reset grading fields)
    DB-->>Server: submission row
    Server-->>Student: 200/201 Submission confirmed
Loading
sequenceDiagram
    participant Host as Host (Frontend)
    participant Server as Server/API
    participant DB as Database

    Host->>Server: GET /api/assignments/:asgnId/submissions
    Server->>Server: verify_token, check host, ensure assignment visible
    Server->>DB: SELECT submissions WHERE assignment_id=asgnId
    DB-->>Server: submissions list
    Server-->>Host: submissions list

    Host->>Server: POST /api/submissions/:subId/grade (score, feedback)
    Server->>Server: verify_token, check host, validate score <= assignment.max_score, encrypt feedback
    Server->>DB: UPDATE submissions SET score, feedback, graded_by, graded_at
    DB-->>Server: updated submission
    Server-->>Host: grading confirmation
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: adding a complete assignment system with submissions and grading capabilities. It is specific, directly related to the changeset, and avoids vague or misleading language.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@public/course.html`:
- Around line 257-260: The current interpolation of a.title into the inline
onclick makes a stored XSS sink; stop embedding user-controlled text inside the
onclick JS string in the hostActions construction and instead place the raw
values into data-* attributes (e.g., data-id and data-title on the button) and
bind the click behavior in script using viewSubmissions/showSubmitForm by
reading dataset.id and dataset.title; update the hostActions generation to not
call esc2 into the onclick, remove inline onclick handlers, and change any code
that expects the inline string to use EventTarget.dataset so escaping is handled
safely and XSS is prevented.
- Around line 411-461: Inputs rely on placeholders and nearby text for
accessible names; add explicit <label for="..."> elements for `#asgn-title`,
`#asgn-desc`, `#asgn-status` and `#grade-score` (and any other form controls that
currently only use placeholders) so screen readers get a stable name; ensure the
label text is descriptive (e.g., "Assignment Title", "Description", "Status",
"Score") and if you want the label visually hidden use the existing project
utility (e.g., an "sr-only" or "visually-hidden" class) rather than removing the
label, keeping the for attribute linked to the corresponding id.
- Around line 388-396: The wrapper for loadActivity currently captures itself
due to hoisting, causing infinite recursion; fix it by preserving the original
loadActivity before you declare/replace it (or by creating the wrapper as an
IIFE that captures the existing function), e.g. first assign const
_origLoadActivity = loadActivity (or const _origLoadActivity =
globalThis.loadActivity) before redefining loadActivity, then in the new async
function call await _origLoadActivity.call(this, ...arguments); and finally
invoke loadAssignments() after that call (or trigger it from DOMContentLoaded);
ensure the unique symbols loadActivity, _origLoadActivity, and loadAssignments
are used exactly as shown so the wrapper delegates to the true original function
rather than itself.

In `@schema.sql`:
- Line 109: Add ON DELETE CASCADE to the submissions.assignment_id foreign key
(and to assignments.activity_id) so deletions are enforced by the DB rather than
the app; update the schema DDL line defining FOREIGN KEY (assignment_id)
REFERENCES assignments(id) to include ON DELETE CASCADE and similarly update
FOREIGN KEY (activity_id) REFERENCES activities(id) for the assignments table,
then mirror these exact changes in the DDL string constant used in
src/worker.py::_DDL so both the SQL schema file and the worker's embedded DDL
stay in sync.

In `@src/worker.py`:
- Around line 1464-1479: The submission handler currently never enforces
r["due_date"] / r["allow_late"]; after confirming enrollment (after the
enrollments query that assigns enr) add a deadline check: parse r["due_date"]
into a timezone-aware datetime (use a robust parser like
dateutil.parser.isoparse or datetime.fromisoformat and, if the result is naive,
attach UTC or your app's timezone) and compare against
datetime.now(timezone-aware); if now > due_date and r["allow_late"] is falsy,
return err("Assignment is closed", 403). Place this check before parsing the
request body (before parse_json_object(req)) so late submissions are rejected
early and ensure you reference r["due_date"] and r["allow_late"] when
implementing the logic.
- Around line 1485-1487: The ON CONFLICT upsert currently replaces
text_response/file_url and sets status='submitted' but leaves grade metadata
stale; in the DO UPDATE clause (the SQL string starting with "ON
CONFLICT(assignment_id, student_id) DO UPDATE SET") also reset grading fields by
setting score=NULL, feedback=NULL, graded_by=NULL, graded_at=NULL and update
submitted_at=datetime('now') so a resubmission appears ungraded; keep the
existing text_response, file_url, status and updated_at updates and add these
fields to the same DO UPDATE SET list.
- Around line 1521-1525: student_name_enc in the mapping is the encrypted
users.name written with the user-module encrypt(), so do not use decrypt_aes()
to decode it; replace the decrypt_aes(s["student_name_enc"] or "", enc) call
with the existing user helper that mirrors encrypt() for users.name (the
user-name decryption helper used elsewhere in the codebase, e.g.,
decrypt_user_name or the user helper that reads users.name), passing the
encrypted value (and omitting/adjusting enc if the helper manages keys) so
student_name is decoded correctly while leaving decrypt_aes for
text_response/file_url unchanged.
- Line 1303: The code calls encrypt_aes() and decrypt_aes() (e.g., in the
assignment endpoints handling description and other fields) but those async AES
functions are not defined; implement them or change callers to use the existing
synchronous encrypt()/decrypt() functions. Fix by either (A) adding async def
encrypt_aes(...)/async def decrypt_aes(...) wrappers that perform AES-GCM
encryption/decryption and return bytes/strings compatible with current callers,
or (B) refactor the handlers that call await encrypt_aes(...) / await
decrypt_aes(...) to call the existing encrypt(...) / decrypt(...) synchronously
(remove await), updating any calling signatures to match; reference functions:
encrypt_aes, decrypt_aes, encrypt, decrypt and the assignment handlers around
the decrypt_aes call shown.
- Around line 1416-1432: The PUT handler currently uses "body.get(...) or
r[...]" which treats missing keys the same as explicit empty values; change each
field to preserve the stored value when the key is absent and only update when
the key is present (treat empty string as an intentional clear). Concretely: for
title/status/max_score/due_date/allow_late/description, check "if 'FIELD' in
body" and then validate/normalize the provided value (e.g. parse and clamp
max_score, validate status in ("draft","published"), convert allow_late to 1/0)
and when description is provided call encrypt_aes(description, enc) (even if
empty) otherwise reuse r["description"]; for values not present keep r[...] (and
for allow_late keep r["allow_late"]). Then bind those computed values into the
env.DB.prepare(...).bind(..., asgn_id).run() so absent keys do not overwrite
existing data.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9124df17-ab63-4c94-9d40-c098b981ad9c

📥 Commits

Reviewing files that changed from the base of the PR and between a08bafc and cb43f33.

📒 Files selected for processing (3)
  • public/course.html
  • schema.sql
  • src/worker.py

@ayesha1145 ayesha1145 force-pushed the feat/assignment-system branch from cb43f33 to 6313c46 Compare March 26, 2026 16:46
@ayesha1145
Copy link
Copy Markdown
Contributor Author

Addressed CodeRabbit feedback:

  • Fixed critical: XSS in assignment buttons — replaced inline onclick with data-id/data-title attributes and delegated click handlers (asgn-view-btn, asgn-submit-btn classes)
  • Fixed critical: infinite recursion in loadActivity wrapper — removed the broken wrapper entirely; assignments now load via DOMContentLoaded listener independently
  • Fixed major: PUT endpoint no longer clobbers absent fields — uses "key" in body checks to distinguish "omitted" from "explicitly set", preserving existing values when keys are absent
  • Fixed major: due date enforcement in submit — checks due_date against current UTC time and rejects late submissions when allow_late=0
  • Fixed major: resubmission clears stale grade — UPSERT now nulls score, feedback, graded_by, graded_at and resets submitted_at on conflict
  • Fixed minor: added ON DELETE CASCADE to submissions.assignment_id and assignments.activity_id in both schema.sql and _DDL
  • Fixed accessibility: added sr-only labels for asgn-title, asgn-desc, and grade-score inputs

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (2)
src/worker.py (2)

1528-1541: ⚠️ Potential issue | 🟠 Major

Encryption function mismatch for student_name.

The users.name column is encrypted using the existing encrypt() function (see lines 432, 617), but here at line 1533 you're attempting to decrypt it with decrypt_aes(). Even after implementing decrypt_aes(), this would fail to properly decode the student name.

Use decrypt() for student_name since that's what was used to encrypt it:

💡 Suggested fix
             "id": s["id"],
             "student_id": s["student_id"],
-            "student_name": await decrypt_aes(s["student_name_enc"] or "", enc),
+            "student_name": decrypt(s["student_name_enc"] or "", enc),
             "text_response": await decrypt_aes(s["text_response"] or "", enc),

Note: text_response, file_url, and feedback are written by the new assignment handlers, so they should use whichever encryption function you choose for assignments (once you implement or switch to it).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` around lines 1528 - 1541, The student_name field is being
decrypted with decrypt_aes() but was originally encrypted with encrypt(), so
replace the decrypt_aes(s["student_name_enc"] or "", enc) call with
decrypt(s["student_name_enc"] or "", enc) in the submissions assembly; leave
decrypt_aes() usage for text_response/file_url/feedback only if and when those
fields are encrypted with the AES scheme, and ensure the change is made where
submissions is populated (the block that builds submissions from rows.results).

1279-1311: ⚠️ Potential issue | 🔴 Critical

Fix undefined encryption functions encrypt_aes and decrypt_aes — code will crash at runtime.

The functions encrypt_aes() and decrypt_aes() are called throughout this function and others (lines 1303, 1347, 1372, 1393, 1417, 1431, 1500, 1501, 1533, 1534, 1535, 1538, 1575) but are never defined. When these endpoints execute, Python will raise NameError: name 'encrypt_aes' is not defined.

The codebase already has synchronous encrypt() and decrypt() functions (lines 87–104) using an XOR stream cipher. Replace all calls to the undefined async functions with the existing synchronous ones. Remember to remove the await keyword since these functions are not async.

Quick fix

Replace all occurrences of:

  • await encrypt_aes(...)encrypt(...)
  • await decrypt_aes(...)decrypt(...)

For example, at line 1303:

-            "description": await decrypt_aes(r["description"] or "", enc),
+            "description": decrypt(r["description"] or "", enc),

Apply the same pattern throughout the file (lines 1303, 1347, 1372, 1393, 1417, 1431, 1500, 1501, 1533, 1534, 1535, 1538, 1575).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` around lines 1279 - 1311, The code calls undefined async
helpers encrypt_aes()/decrypt_aes() (e.g., in api_list_assignments) causing
NameError; replace all calls to await encrypt_aes(...) with the existing
synchronous encrypt(...) and all calls to await decrypt_aes(...) with
decrypt(...), and remove the await keyword where used (search for
encrypt_aes/decrypt_aes across the file and update each call), ensuring
functions like api_list_assignments, and other handlers listed in the review,
now call encrypt/decrypt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@public/course.html`:
- Line 412: The button with id "btn-create-assignment" that calls
showCreateAssignment() should explicitly include type="button" to avoid acting
as a submit button in any enclosing form; update the <button
id="btn-create-assignment"...> element to add the type attribute set to "button"
so clicking it only triggers showCreateAssignment() and not a form submission.
- Around line 370-386: In gradeSubmission(), after a successful POST (res.ok)
and hideGradeForm(), call the submissions refresh routine so the UI reflects the
new score — e.g. invoke viewSubmissions(currentViewingAsgnId) or the equivalent
function that reloads the submissions list; if currentViewingAsgnId isn't
tracked yet, ensure you store the active assignment id when opening submissions
(a module/global like currentViewingAsgnId) and use it here to refresh the list
after grading.
- Around line 338-358: In viewSubmissions, stop injecting an inline onclick with
s.id to avoid XSS; instead render the Grade button with a data-id attribute
(e.g., data-id="' + esc2(s.id) + '") and a class such as "sub-grade-btn"
(matching existing patterns used for assignment buttons), and remove the inline
showGradeForm(...) string. Then wire up an event-delegation handler on the
parent container (the element with id "assignments-list") that listens for
clicks, checks e.target.classList.contains('sub-grade-btn'), and calls
showGradeForm(e.target.dataset.id). Continue to use esc2 for any interpolated
user content like s.student_name and s.text_response when building the HTML.
- Around line 423-430: Add proper label/input associations by giving each label
a matching for attribute that references the input id: update the "Due Date"
label to for="asgn-due" and the "Max Score" label to for="asgn-score" so screen
readers and assistive tech can link the labels to the inputs with ids asgn-due
and asgn-score respectively; ensure the id values remain unchanged and the
labels retain their existing classes and text.
- Around line 450-457: Add visually hidden (sr-only) <label> elements for the
submission textarea and the grade feedback control so screen readers always have
a persistent label; specifically, add a label element with class "sr-only" and
for="submit-text" and meaningful text like "Assignment response" next to the
<textarea id="submit-text"> inside the submit-form block, and likewise add an
sr-only label for the `#grade-feedback` control (use its id in the for attribute
or set aria-labelledby) to ensure proper association; update any JS that
references labels by id if needed (e.g., submitAssignment(), hideSubmitForm())
so behavior is unchanged.

In `@schema.sql`:
- Around line 97-110: Add database-level CHECK constraints to the assignments
table to enforce allowed values: update the assignments table definition
(referencing the assignments table) to add a CHECK on the status column (status
IN ('draft','published')) and a CHECK on max_score (max_score > 0 AND max_score
<= 1000) so the columns status and max_score are validated by the database.

---

Duplicate comments:
In `@src/worker.py`:
- Around line 1528-1541: The student_name field is being decrypted with
decrypt_aes() but was originally encrypted with encrypt(), so replace the
decrypt_aes(s["student_name_enc"] or "", enc) call with
decrypt(s["student_name_enc"] or "", enc) in the submissions assembly; leave
decrypt_aes() usage for text_response/file_url/feedback only if and when those
fields are encrypted with the AES scheme, and ensure the change is made where
submissions is populated (the block that builds submissions from rows.results).
- Around line 1279-1311: The code calls undefined async helpers
encrypt_aes()/decrypt_aes() (e.g., in api_list_assignments) causing NameError;
replace all calls to await encrypt_aes(...) with the existing synchronous
encrypt(...) and all calls to await decrypt_aes(...) with decrypt(...), and
remove the await keyword where used (search for encrypt_aes/decrypt_aes across
the file and update each call), ensuring functions like api_list_assignments,
and other handlers listed in the review, now call encrypt/decrypt.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 912d881b-f27c-4299-9e6d-d744d2bb9566

📥 Commits

Reviewing files that changed from the base of the PR and between cb43f33 and 6313c46.

📒 Files selected for processing (3)
  • public/course.html
  • schema.sql
  • src/worker.py

@ayesha1145 ayesha1145 force-pushed the feat/assignment-system branch from 6313c46 to b8669be Compare March 26, 2026 17:26
@ayesha1145
Copy link
Copy Markdown
Contributor Author

Addressed latest CodeRabbit feedback:

  • Fixed minor: XSS in grade button — replaced inline onclick with grade-btn class and data-id attribute; added delegated click handler alongside existing asgn-view-btn/asgn-submit-btn handlers
  • Fixed nitpick: submissions view now refreshes after grading — viewSubmissions() called after successful grade save
  • Fixed nitpick: added type="button" to New Assignment button
  • Fixed minor: added for attributes to Due Date (asgn-due) and Max Score (asgn-score) labels for screen reader association
  • Fixed nitpick: added sr-only label for submit-text textarea

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (2)
public/course.html (1)

467-469: 🧹 Nitpick | 🔵 Trivial

Consider adding sr-only label for grade-feedback textarea.

The #grade-feedback textarea relies on placeholder text for context. For consistency with other form controls (like #submit-text which has an sr-only label), consider adding one here too.

💡 Suggested improvement
+      <label for="grade-feedback" class="sr-only">Feedback</label>
       <textarea id="grade-feedback" rows="3" placeholder="Feedback (optional)" class="w-full border border-slate-200 rounded-lg px-3 py-2 text-sm mb-2 focus:outline-none focus:ring-2 focus:ring-indigo-300 resize-none"></textarea>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/course.html` around lines 467 - 469, Add a hidden (sr-only) label for
the feedback textarea so it has the same accessible pattern as other controls:
create a <label> element with class "sr-only" and for="grade-feedback" (text
such as "Feedback") placed near the existing <textarea id="grade-feedback"> so
screen readers have a proper label while keeping the placeholder intact; ensure
the label text is concise and matches the other sr-only labels used in the form.
src/worker.py (1)

1303-1303: ⚠️ Potential issue | 🔴 Critical

Critical: encrypt_aes and decrypt_aes functions are undefined and will cause runtime errors.

All assignment and submission endpoints call encrypt_aes() and decrypt_aes() (across 13+ locations), but these functions don't exist in the codebase. This will cause NameError exceptions at runtime, breaking all assignment/submission functionality.

The codebase provides synchronous encrypt() and decrypt() functions (lines 87-114) that use XOR stream cipher. You have two options:

  1. Use existing functions (quick fix): Replace all occurrences of encrypt_aes(...) and decrypt_aes(...) with encrypt(...) and decrypt(...), and remove the await keywords since these are synchronous.

  2. Implement AES-GCM (production-ready): Add proper async AES encryption functions.

🔧 Quick fix: Use existing encrypt/decrypt functions

Replace all await decrypt_aes(...) with decrypt(...) and all await encrypt_aes(...) with encrypt(...). For example:

-            "description": await decrypt_aes(r["description"] or "", enc),
+            "description": decrypt(r["description"] or "", enc),

Apply to all 13+ occurrences across lines 1303, 1347, 1372, 1393, 1417, 1431, 1500–1501, 1533–1535, 1538, and 1575.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` at line 1303, The runtime error is caused by calls to
undefined async helpers encrypt_aes(...) and decrypt_aes(...); replace every use
of encrypt_aes and decrypt_aes with the existing synchronous encrypt(...) and
decrypt(...) functions (remove the await keywords since encrypt/decrypt are
synchronous XOR helpers), ensuring all 13+ call sites (e.g., where
"description": await decrypt_aes(...) is used) now call decrypt(...) or
encrypt(...); alternatively, if you prefer a production fix implement proper
AES-GCM async helpers named encrypt_aes/decrypt_aes and use them instead, but
the quick fix is to swap to encrypt/decrypt and drop await.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@public/course.html`:
- Around line 381-384: The bug is that hideGradeForm() clears currentSubId
before you attempt to refresh, so the subsequent if (currentSubId) check will
always be false; fix it by preserving the id to refresh before calling
hideGradeForm (e.g., const refreshedAsgnId = currentSubId), then call
hideGradeForm(), then call viewSubmissions(refreshedAsgnId) if it exists, or
alternately implement a separate tracker (currentViewingAsgnId) updated when
viewing submissions and call viewSubmissions(currentViewingAsgnId) after
grading; adjust the logic around the existing hideGradeForm, currentSubId and
viewSubmissions functions accordingly.
- Around line 441-443: The checkbox lacks an explicit label linkage: update the
label element that wraps the input with a for="asgn-late" attribute (targeting
the input id "asgn-late") so assistive tech can reliably associate the text
"Allow late submissions" with the checkbox; alternatively, move the <input
id="asgn-late"> outside the label and add the for attribute on the label
pointing to "asgn-late" to achieve the same effect.

In `@schema.sql`:
- Line 109: The schema uses ON DELETE CASCADE which is enforced in Cloudflare D1
but may be disabled in local SQLite; update the database initialization in
init_db() to explicitly enable foreign keys so cascade behavior is consistent
across environments by running the equivalent of PRAGMA foreign_keys = ON before
executing the _DDL statements (call this once at the start of init_db() prior to
preparing/executing the SQL list) to ensure local tests and documentation match
D1 behavior.

In `@src/worker.py`:
- Around line 1473-1481: Replace the broad except in the due date enforcement
block so only datetime parsing errors are ignored and all other exceptions are
logged: catch ValueError (from datetime.fromisoformat on r["due_date"]) and let
the submission proceed, but add a separate except Exception that logs the
unexpected error using the module's logger (e.g., process_logger or app logger)
before continuing; keep the existing early return via err(...) when the parsed
due datetime shows the assignment is past due and allow_late is false.

---

Duplicate comments:
In `@public/course.html`:
- Around line 467-469: Add a hidden (sr-only) label for the feedback textarea so
it has the same accessible pattern as other controls: create a <label> element
with class "sr-only" and for="grade-feedback" (text such as "Feedback") placed
near the existing <textarea id="grade-feedback"> so screen readers have a proper
label while keeping the placeholder intact; ensure the label text is concise and
matches the other sr-only labels used in the form.

In `@src/worker.py`:
- Line 1303: The runtime error is caused by calls to undefined async helpers
encrypt_aes(...) and decrypt_aes(...); replace every use of encrypt_aes and
decrypt_aes with the existing synchronous encrypt(...) and decrypt(...)
functions (remove the await keywords since encrypt/decrypt are synchronous XOR
helpers), ensuring all 13+ call sites (e.g., where "description": await
decrypt_aes(...) is used) now call decrypt(...) or encrypt(...); alternatively,
if you prefer a production fix implement proper AES-GCM async helpers named
encrypt_aes/decrypt_aes and use them instead, but the quick fix is to swap to
encrypt/decrypt and drop await.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a07f535c-bf5e-4ca5-8763-92f3d955fd07

📥 Commits

Reviewing files that changed from the base of the PR and between 6313c46 and b8669be.

📒 Files selected for processing (3)
  • public/course.html
  • schema.sql
  • src/worker.py

@ayesha1145 ayesha1145 force-pushed the feat/assignment-system branch from b8669be to 5418993 Compare March 26, 2026 17:43
@ayesha1145
Copy link
Copy Markdown
Contributor Author

Addressed latest CodeRabbit feedback:

  • Fixed minor: saved currentSubId before hideGradeForm() nulls it — viewSubmissions() now correctly refreshes after grading
  • Fixed minor: added for="asgn-late" to the Allow late submissions label for proper screen reader association
  • Fixed nitpick: due date parse errors now logged via capture_exception() instead of silently swallowed
  • PRAGMA foreign_keys: acknowledged — D1 enforces FK constraints automatically, no pragma needed

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (6)
src/worker.py (3)

1530-1533: ⚠️ Potential issue | 🟠 Major

Decrypt student_name_enc with decrypt(), not the assignment cipher.

users.name is still stored via encrypt(...) in this module. Even after decrypt_aes() exists, Line 1533 will decode the wrong format and host views will show broken student names.

🛠️ Targeted fix
-            "student_name": await decrypt_aes(s["student_name_enc"] or "", enc),
+            "student_name": decrypt(s["student_name_enc"] or "", enc),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` around lines 1530 - 1533, The student_name_enc field is being
decoded with decrypt_aes (in the submissions.append block) but the stored
users.name was encrypted with encrypt and must be decoded with decrypt; update
the submissions construction to call decrypt(student_name_enc or "", enc)
instead of decrypt_aes(...), ensuring you reference the decrypt function and the
submissions.append location so host views render correct student names.

1473-1482: ⚠️ Potential issue | 🔴 Critical

Normalize timezone-less deadlines before comparing them to UTC.

The current UI posts datetime-local values like 2026-03-30T10:00 without an offset. On Line 1477, datetime.fromisoformat(...) returns a naive datetime for that input, so Line 1478 raises TypeError against datetime.now(timezone.utc). Any published assignment with allow_late=0 and a due date can 500 on submit instead of returning the intended 403.

🛠️ Minimum safe normalization
     if r["due_date"] and not r["allow_late"]:
         from datetime import datetime, timezone
         try:
             due = datetime.fromisoformat(r["due_date"].replace("Z", "+00:00"))
+            if due.tzinfo is None:
+                due = due.replace(tzinfo=timezone.utc)
             if datetime.now(timezone.utc) > due:
                 return err("This assignment is past due and does not allow late submissions", 403)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` around lines 1473 - 1482, The code parses r["due_date"] with
datetime.fromisoformat which returns a naive datetime for timezone-less strings
and then compares it to datetime.now(timezone.utc), causing a TypeError; update
the parsing in the api_submit_assignment block so that after creating due via
datetime.fromisoformat(r["due_date"].replace("Z", "+00:00")), you detect if
due.tzinfo is None and if so attach UTC (e.g., due =
due.replace(tzinfo=timezone.utc)) before comparing to
datetime.now(timezone.utc); keep the existing exception capture
(capture_exception) and the err(...) return behavior unchanged.

1303-1303: ⚠️ Potential issue | 🔴 Critical

Define encrypt_aes() / decrypt_aes() or switch these callers consistently.

The module still only defines encrypt() / decrypt() at Lines 87-114. The first request through any of these create/read/submit/grade branches will fail with NameError.

Also applies to: 1347-1347, 1372-1372, 1393-1393, 1417-1417, 1431-1431, 1500-1501, 1534-1535, 1538-1538, 1575-1575

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` at line 1303, The code calls decrypt_aes()/encrypt_aes() in
multiple locations (e.g., the object deserialization line that uses decrypt_aes
for "description") but the module only defines encrypt()/decrypt(); this causes
NameError on first use—either add thin wrapper functions named encrypt_aes(data,
enc) and decrypt_aes(data, enc) that delegate to the existing
encrypt()/decrypt(), or change all callers (e.g., where decrypt_aes and
encrypt_aes are referenced) to call encrypt()/decrypt() consistently; update or
add the functions near the existing encrypt()/decrypt() definitions (Lines
~87-114) so all usages resolve.
public/course.html (3)

381-385: ⚠️ Potential issue | 🟡 Minor

Refresh submissions with the assignment id, not the submission id.

gradedSubId is copied from currentSubId, but viewSubmissions() expects an assignment id. After a successful grade, this ends up requesting /api/assignments/<submission-id>/submissions, so the list still stays stale.

🛠️ Track the currently viewed assignment
   let currentAsgnId = null;
   let currentSubId = null;
+  let currentViewingAsgnId = null;
...
   async function viewSubmissions(asgnId) {
+    currentViewingAsgnId = asgnId;
     try {
...
-        const gradedSubId = currentSubId;
+        const asgnIdToRefresh = currentViewingAsgnId;
         hideGradeForm();
         alert('Graded successfully!');
-        if (gradedSubId) viewSubmissions(gradedSubId);
+        if (asgnIdToRefresh) await viewSubmissions(asgnIdToRefresh);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/course.html` around lines 381 - 385, The code is passing a submission
id to viewSubmissions() (gradedSubId = currentSubId) but viewSubmissions()
expects an assignment id; update the handler so after a successful grade you
call viewSubmissions() with the assignment id instead of the submission id.
Specifically, replace usage of gradedSubId/currentSubId with the currently
tracked assignment id (e.g., currentAssignmentId) or derive the assignment id
from the current submission object (e.g., currentSubmission.assignmentId) and
pass that into viewSubmissions(assignmentId); keep the hideGradeForm() and
success alert flow unchanged.

257-259: ⚠️ Potential issue | 🔴 Critical

Escape data-title for attribute context.

esc2() only makes a value safe for text nodes. It does not escape quotes, so a title containing " can break out of data-title="..." when this string is assigned to innerHTML, which reopens a stored XSS path here.

As per coding guidelines, **/*.html: Review HTML templates for accessibility (ARIA attributes, semantic elements), XSS risks from unescaped user content, and proper use of template inheritance.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/course.html` around lines 257 - 259, The hostActions string builds an
HTML attribute data-title using esc2(a.title), which only escapes text-node
context and can leave quotes unescaped causing attribute-level XSS; replace
esc2(a.title) with an attribute-escaping function (e.g., escAttr or a new helper
that escapes &, ", ', <, >) when constructing the string in the assignment that
produces hostActions (refer to the hostActions variable and esc2 usage) so
data-title is safe for inclusion inside quotes when the HTML is later set via
innerHTML.

438-443: ⚠️ Potential issue | 🟡 Minor

Label the remaining assignment controls.

#asgn-status and #grade-feedback still rely on surrounding text or placeholder text, so they do not have stable accessible names for assistive tech. Please add explicit <label for> elements like the other new inputs.

🛠️ Minimal label additions
       <div class="flex items-center gap-4 mb-3">
+        <label for="asgn-status" class="sr-only">Assignment status</label>
         <select id="asgn-status" class="border border-slate-200 rounded-lg px-3 py-2 text-sm focus:outline-none focus:ring-2 focus:ring-indigo-300">
...
+      <label for="grade-feedback" class="sr-only">Feedback</label>
       <textarea id="grade-feedback" rows="3" placeholder="Feedback (optional)" class="w-full border border-slate-200 rounded-lg px-3 py-2 text-sm mb-2 focus:outline-none focus:ring-2 focus:ring-indigo-300 resize-none"></textarea>
As per coding guidelines, `**/*.html`: Review HTML templates for accessibility (ARIA attributes, semantic elements), XSS risks from unescaped user content, and proper use of template inheritance.

Also applies to: 466-470

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/course.html` around lines 438 - 443, Add explicit <label for="...">
elements for the remaining assignment controls so they have stable accessible
names: add a label element tied to id="asgn-status" (e.g., "Assignment status")
and another label tied to id="grade-feedback" (e.g., "Grade / Feedback")
following the pattern used for other inputs (use the for attribute matching the
input id and the same styling/placement as existing labels), and ensure the
existing checkbox input id="asgn-late" also has a corresponding <label
for="asgn-late"> if not already present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@public/course.html`:
- Around line 403-405: Assignment loading can race with activity loading because
renderAssignments relies on isHost which is only set in loadActivity; ensure
loadAssignments runs after loadActivity completes by sequencing them (e.g., have
loadActivity return a Promise and call await loadActivity() before calling
loadAssignments(), or call loadAssignments from its .then callback). Update the
DOMContentLoaded handler so it invokes loadActivity() first and only calls
loadAssignments() after loadActivity finishes (or guard renderAssignments to
wait for isHost), referencing loadActivity, loadAssignments, renderAssignments,
isHost and the DOMContentLoaded listener to locate where to change.

In `@src/worker.py`:
- Around line 364-398: The embedded DDL in _DDL used by init_db() is out of sync
with schema.sql: update the sessions table entry in the _DDL list so its FOREIGN
KEY (activity_id) includes ON DELETE CASCADE (matching schema.sql Line 45);
locate the sessions CREATE TABLE string in src/worker.py (within the _DDL list
used by init_db()) and add the "ON DELETE CASCADE" clause to the activity_id
foreign key definition so databases created via /api/init and /api/seed have the
same delete behavior.

---

Duplicate comments:
In `@public/course.html`:
- Around line 381-385: The code is passing a submission id to viewSubmissions()
(gradedSubId = currentSubId) but viewSubmissions() expects an assignment id;
update the handler so after a successful grade you call viewSubmissions() with
the assignment id instead of the submission id. Specifically, replace usage of
gradedSubId/currentSubId with the currently tracked assignment id (e.g.,
currentAssignmentId) or derive the assignment id from the current submission
object (e.g., currentSubmission.assignmentId) and pass that into
viewSubmissions(assignmentId); keep the hideGradeForm() and success alert flow
unchanged.
- Around line 257-259: The hostActions string builds an HTML attribute
data-title using esc2(a.title), which only escapes text-node context and can
leave quotes unescaped causing attribute-level XSS; replace esc2(a.title) with
an attribute-escaping function (e.g., escAttr or a new helper that escapes &, ",
', <, >) when constructing the string in the assignment that produces
hostActions (refer to the hostActions variable and esc2 usage) so data-title is
safe for inclusion inside quotes when the HTML is later set via innerHTML.
- Around line 438-443: Add explicit <label for="..."> elements for the remaining
assignment controls so they have stable accessible names: add a label element
tied to id="asgn-status" (e.g., "Assignment status") and another label tied to
id="grade-feedback" (e.g., "Grade / Feedback") following the pattern used for
other inputs (use the for attribute matching the input id and the same
styling/placement as existing labels), and ensure the existing checkbox input
id="asgn-late" also has a corresponding <label for="asgn-late"> if not already
present.

In `@src/worker.py`:
- Around line 1530-1533: The student_name_enc field is being decoded with
decrypt_aes (in the submissions.append block) but the stored users.name was
encrypted with encrypt and must be decoded with decrypt; update the submissions
construction to call decrypt(student_name_enc or "", enc) instead of
decrypt_aes(...), ensuring you reference the decrypt function and the
submissions.append location so host views render correct student names.
- Around line 1473-1482: The code parses r["due_date"] with
datetime.fromisoformat which returns a naive datetime for timezone-less strings
and then compares it to datetime.now(timezone.utc), causing a TypeError; update
the parsing in the api_submit_assignment block so that after creating due via
datetime.fromisoformat(r["due_date"].replace("Z", "+00:00")), you detect if
due.tzinfo is None and if so attach UTC (e.g., due =
due.replace(tzinfo=timezone.utc)) before comparing to
datetime.now(timezone.utc); keep the existing exception capture
(capture_exception) and the err(...) return behavior unchanged.
- Line 1303: The code calls decrypt_aes()/encrypt_aes() in multiple locations
(e.g., the object deserialization line that uses decrypt_aes for "description")
but the module only defines encrypt()/decrypt(); this causes NameError on first
use—either add thin wrapper functions named encrypt_aes(data, enc) and
decrypt_aes(data, enc) that delegate to the existing encrypt()/decrypt(), or
change all callers (e.g., where decrypt_aes and encrypt_aes are referenced) to
call encrypt()/decrypt() consistently; update or add the functions near the
existing encrypt()/decrypt() definitions (Lines ~87-114) so all usages resolve.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 81d61ac9-ef64-477c-891d-5494e4468fa4

📥 Commits

Reviewing files that changed from the base of the PR and between b8669be and 5418993.

📒 Files selected for processing (3)
  • public/course.html
  • schema.sql
  • src/worker.py

@ayesha1145 ayesha1145 force-pushed the feat/assignment-system branch from 5418993 to dd36a05 Compare March 26, 2026 18:04
@ayesha1145
Copy link
Copy Markdown
Contributor Author

Addressed latest CodeRabbit feedback:

  • Fixed major: loadAssignments() now runs after loadActivity() completes — wrapped window.loadActivity to ensure isHost is set before renderAssignments() runs
  • Fixed major: synced sessions table FK in _DDL with schema.sql — added ON DELETE CASCADE to sessions.activity_id so /api/init matches schema.sql

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (6)
public/course.html (3)

234-234: ⚠️ Potential issue | 🔴 Critical

data-title is still an attribute-context XSS sink.

esc2() is only safe for text nodes. It does not escape ", so a crafted assignment title can break out of data-title and inject extra attributes or handlers into the button element. Use an attribute-safe encoder here, or set dataset.title via DOM APIs instead of concatenating HTML.

🔐 Minimal safe fix
   function esc2(s) { const d = document.createElement('div'); d.textContent = s||''; return d.innerHTML; }
+  function escAttr(s) {
+    return String(s == null ? '' : s)
+      .replace(/&/g, '&amp;')
+      .replace(/"/g, '&quot;')
+      .replace(/'/g, '&#39;')
+      .replace(/</g, '&lt;')
+      .replace(/>/g, '&gt;');
+  }
...
-        : (token ? '<button type="button" class="asgn-submit-btn text-xs text-indigo-500 hover:underline" data-id="' + a.id + '" data-title="' + esc2(a.title) + '">Submit</button>' : '');
+        : (token ? '<button type="button" class="asgn-submit-btn text-xs text-indigo-500 hover:underline" data-id="' + a.id + '" data-title="' + escAttr(a.title) + '">Submit</button>' : '');

As per coding guidelines, **/*.html: Review HTML templates for accessibility (ARIA attributes, semantic elements), XSS risks from unescaped user content, and proper use of template inheritance.

Also applies to: 257-259

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/course.html` at line 234, The current esc2(s) function only escapes
text-node context and does not encode quotes, leaving the data-title attribute
as an XSS sink; instead of inserting via concatenated HTML, set the attribute
using DOM APIs (e.g., element.dataset.title = title) or replace esc2 with an
attribute-safe encoder before writing into data-title; update all uses that
build data-title (references to esc2 and the code that concatenates the
button/element HTML) to assign the title via element.dataset.title or a proper
attribute encoder to prevent attribute-context injection.

338-358: ⚠️ Potential issue | 🟠 Major

Refresh with the assignment ID, not currentSubId.

showGradeForm() only stores the submission ID. After a successful grade, viewSubmissions(gradedSubId) calls the assignment-submissions endpoint with a submission ID, so the UI follows a successful save with a failing refresh. Track the active assignment separately.

🔄 Suggested state fix
   let currentAsgnId = null;
   let currentSubId = null;
+  let currentViewingAsgnId = null;
   let isHost = false;
...
   async function viewSubmissions(asgnId) {
+    currentViewingAsgnId = asgnId;
     try {
...
       if (res.ok) {
-        const gradedSubId = currentSubId;
+        const asgnIdToRefresh = currentViewingAsgnId;
         hideGradeForm();
         alert('Graded successfully!');
-        if (gradedSubId) viewSubmissions(gradedSubId);
+        if (asgnIdToRefresh) await viewSubmissions(asgnIdToRefresh);
       } else { alert(data.error || 'Failed'); }

Also applies to: 360-385

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/course.html` around lines 338 - 358, The refresh is calling
viewSubmissions with a submission id instead of the assignment id; update state
so the active assignment id is tracked and used for refresh: when opening
submissions (in viewSubmissions or the code that calls showGradeForm) store the
assignment id in a dedicated variable (e.g., currentAssignId) or pass the
assignmentId into showGradeForm, and after grading call
viewSubmissions(currentAssignId) (not currentSubId or gradedSubId). Ensure
functions referenced are viewSubmissions and showGradeForm and replace any
places that call viewSubmissions(gradedSubId) with
viewSubmissions(currentAssignId).

440-473: ⚠️ Potential issue | 🟠 Major

Add labels for the status select and feedback field.

#asgn-status and #grade-feedback still rely on adjacent text or placeholder text, so they do not have stable accessible names for screen readers.

♿ Minimal label additions
       <div class="flex items-center gap-4 mb-3">
+        <label for="asgn-status" class="sr-only">Assignment status</label>
         <select id="asgn-status" class="border border-slate-200 rounded-lg px-3 py-2 text-sm focus:outline-none focus:ring-2 focus:ring-indigo-300">
           <option value="draft">Draft</option>
           <option value="published">Published</option>
         </select>
...
       <label for="grade-score" class="sr-only">Score</label>
       <input id="grade-score" type="number" min="0" placeholder="Score" class="w-full border border-slate-200 rounded-lg px-3 py-2 text-sm mb-2 focus:outline-none focus:ring-2 focus:ring-indigo-300">
+      <label for="grade-feedback" class="sr-only">Feedback</label>
       <textarea id="grade-feedback" rows="3" placeholder="Feedback (optional)" class="w-full border border-slate-200 rounded-lg px-3 py-2 text-sm mb-2 focus:outline-none focus:ring-2 focus:ring-indigo-300 resize-none"></textarea>

As per coding guidelines, **/*.html: Review HTML templates for accessibility (ARIA attributes, semantic elements), XSS risks from unescaped user content, and proper use of template inheritance.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/course.html` around lines 440 - 473, Add explicit labels for the
status select and feedback textarea so screen readers get stable names: create
<label for="asgn-status"> (e.g., "Status") for the select with id="asgn-status"
and a <label for="grade-feedback"> (e.g., "Feedback") for the textarea with
id="grade-feedback"; if you need to preserve visual layout, use the existing
"sr-only" utility class so the labels are available to assistive tech but hidden
visually. Ensure the for attributes exactly match the element ids (`#asgn-status`
and `#grade-feedback`) and don't rely solely on placeholders.
src/worker.py (3)

1303-1303: ⚠️ Potential issue | 🔴 Critical

These endpoints still call undefined encrypt_aes / decrypt_aes helpers.

This module only defines encrypt() / decrypt(). Any assignment path that touches encrypted fields will raise NameError on first use until these helpers are implemented or the call sites are switched consistently.

Also applies to: 1347-1348, 1372-1372, 1393-1393, 1417-1417, 1431-1432, 1500-1501, 1575-1576

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` at line 1303, The code is calling undefined helpers
encrypt_aes/decrypt_aes which will raise NameError; update all call sites (e.g.,
the mapping that sets "description": await decrypt_aes(...), and the other
occurrences listed around lines ~1347, 1372, 1393, 1417, 1431-1432, 1500-1501,
1575-1576) to use the existing encrypt()/decrypt() functions (or alternatively
implement encrypt_aes/decrypt_aes and wire them to encrypt()/decrypt()); ensure
the calls and awaited usage match the async/sync nature of encrypt()/decrypt()
and adjust imports/exports accordingly so no undefined names remain.

1530-1535: ⚠️ Potential issue | 🟠 Major

Use the user-field decryptor for student_name_enc.

student_name_enc comes from users.name, which is written with encrypt(), not the assignment-field cipher. Even after the AES helpers exist, decrypt_aes() will decode the wrong format here and hosts will not see student names correctly.

🔓 Minimal fix
-            "student_name": await decrypt_aes(s["student_name_enc"] or "", enc),
+            "student_name": decrypt(s["student_name_enc"] or "", enc),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` around lines 1530 - 1535, student_name_enc is encrypted with
the user-field cipher, not the assignment-field AES helper, so replace the call
to decrypt_aes for student_name_enc in the submissions construction with the
user-field decryptor (e.g., use decrypt_user_field or the project’s user-field
decrypt function) while keeping the existing AES decryption for text_response
and file_url; update the expression that currently reads
decrypt_aes(s["student_name_enc"] or "", enc) to call the correct user-field
decryptor with the same null/empty handling so hosts see the proper student
names.

1332-1332: ⚠️ Potential issue | 🔴 Critical

Timezone-less due dates will break the late-submission check.

datetime-local values arrive without an offset, so datetime.fromisoformat() returns a naive datetime. Comparing that to datetime.now(timezone.utc) raises TypeError, which turns overdue submissions into a 500 instead of a 403. Normalize due dates to an absolute timestamp before storing/comparing, or reject naive values.

Run this to confirm the client is sending datetime-local values and the current comparison fails on naive datetimes:

#!/bin/bash
set -e

echo "Due-date input in the client:"
rg -n 'id="asgn-due"|type="datetime-local"' public/course.html -C1

echo
python - <<'PY'
from datetime import datetime, timezone

due = datetime.fromisoformat("2026-03-26T17:00")
print("parsed tzinfo:", due.tzinfo)

try:
    print(datetime.now(timezone.utc) > due)
except Exception as exc:
    print(type(exc).__name__ + ":", exc)
PY

As per coding guidelines, **/*.py: Review Python code for PEP 8 compliance, proper type hints, and Django best practices where applicable. Check for security vulnerabilities such as SQL injection, XSS, and insecure use of eval(). Ensure proper error handling and logging. Verify tests cover the key logic paths.

Also applies to: 1418-1418, 1473-1481

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/worker.py` at line 1332, The stored due_date is parsed as a naive
datetime (from the line setting due_date = (body.get("due_date") or "").strip()
or None) which causes TypeError when compared to datetime.now(timezone.utc); fix
by normalizing parsed values to an aware UTC timestamp: parse with
datetime.fromisoformat(), and if the resulting dt.tzinfo is None, set dt =
dt.replace(tzinfo=timezone.utc) (or reject the input) and then convert to UTC
via dt = dt.astimezone(timezone.utc) before storing/comparing; update any code
paths that compare due_date to datetime.now(timezone.utc) (and any
functions/methods that read due_date) to expect an aware UTC datetime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@public/course.html`:
- Around line 416-479: Move the entire assignments block (the div with
id="assignments-section" containing btn-create-assignment,
create-assignment-form, assignments-list, submit-form, and grade-form) into the
main document flow by placing it inside the <main> element and before the
<footer>; update any code that assumes its current position if needed and ensure
all onclick handlers referenced (showCreateAssignment, createAssignment,
hideCreateAssignment, submitAssignment, hideSubmitForm, gradeSubmission,
hideGradeForm) still resolve. While relocating, add/verify appropriate semantic
landmarks and ARIA where needed (e.g., role="region" and aria-labelledby on the
assignments section header) and ensure any user-provided content inserted into
assignments-list or submit-asgn-title is safely escaped/sanitized to avoid XSS.

In `@src/worker.py`:
- Around line 1449-1450: The two-step deletion (explicitly calling
env.DB.prepare("DELETE FROM submissions WHERE
assignment_id=?").bind(asgn_id).run() then env.DB.prepare("DELETE FROM
assignments WHERE id=?").bind(asgn_id).run()) should be simplified to a single
delete of the assignment row so the submissions FK cascade handles child rows;
remove the explicit submissions DELETE and only execute the assignments DELETE
using asgn_id (or wrap the single DELETE in a transaction if desired), ensuring
you no longer perform the separate submissions deletion to avoid the
partial-failure window.

---

Duplicate comments:
In `@public/course.html`:
- Line 234: The current esc2(s) function only escapes text-node context and does
not encode quotes, leaving the data-title attribute as an XSS sink; instead of
inserting via concatenated HTML, set the attribute using DOM APIs (e.g.,
element.dataset.title = title) or replace esc2 with an attribute-safe encoder
before writing into data-title; update all uses that build data-title
(references to esc2 and the code that concatenates the button/element HTML) to
assign the title via element.dataset.title or a proper attribute encoder to
prevent attribute-context injection.
- Around line 338-358: The refresh is calling viewSubmissions with a submission
id instead of the assignment id; update state so the active assignment id is
tracked and used for refresh: when opening submissions (in viewSubmissions or
the code that calls showGradeForm) store the assignment id in a dedicated
variable (e.g., currentAssignId) or pass the assignmentId into showGradeForm,
and after grading call viewSubmissions(currentAssignId) (not currentSubId or
gradedSubId). Ensure functions referenced are viewSubmissions and showGradeForm
and replace any places that call viewSubmissions(gradedSubId) with
viewSubmissions(currentAssignId).
- Around line 440-473: Add explicit labels for the status select and feedback
textarea so screen readers get stable names: create <label for="asgn-status">
(e.g., "Status") for the select with id="asgn-status" and a <label
for="grade-feedback"> (e.g., "Feedback") for the textarea with
id="grade-feedback"; if you need to preserve visual layout, use the existing
"sr-only" utility class so the labels are available to assistive tech but hidden
visually. Ensure the for attributes exactly match the element ids (`#asgn-status`
and `#grade-feedback`) and don't rely solely on placeholders.

In `@src/worker.py`:
- Line 1303: The code is calling undefined helpers encrypt_aes/decrypt_aes which
will raise NameError; update all call sites (e.g., the mapping that sets
"description": await decrypt_aes(...), and the other occurrences listed around
lines ~1347, 1372, 1393, 1417, 1431-1432, 1500-1501, 1575-1576) to use the
existing encrypt()/decrypt() functions (or alternatively implement
encrypt_aes/decrypt_aes and wire them to encrypt()/decrypt()); ensure the calls
and awaited usage match the async/sync nature of encrypt()/decrypt() and adjust
imports/exports accordingly so no undefined names remain.
- Around line 1530-1535: student_name_enc is encrypted with the user-field
cipher, not the assignment-field AES helper, so replace the call to decrypt_aes
for student_name_enc in the submissions construction with the user-field
decryptor (e.g., use decrypt_user_field or the project’s user-field decrypt
function) while keeping the existing AES decryption for text_response and
file_url; update the expression that currently reads
decrypt_aes(s["student_name_enc"] or "", enc) to call the correct user-field
decryptor with the same null/empty handling so hosts see the proper student
names.
- Line 1332: The stored due_date is parsed as a naive datetime (from the line
setting due_date = (body.get("due_date") or "").strip() or None) which causes
TypeError when compared to datetime.now(timezone.utc); fix by normalizing parsed
values to an aware UTC timestamp: parse with datetime.fromisoformat(), and if
the resulting dt.tzinfo is None, set dt = dt.replace(tzinfo=timezone.utc) (or
reject the input) and then convert to UTC via dt = dt.astimezone(timezone.utc)
before storing/comparing; update any code paths that compare due_date to
datetime.now(timezone.utc) (and any functions/methods that read due_date) to
expect an aware UTC datetime.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7633f519-e86f-44dd-abbe-3ba43a114954

📥 Commits

Reviewing files that changed from the base of the PR and between 5418993 and dd36a05.

📒 Files selected for processing (3)
  • public/course.html
  • schema.sql
  • src/worker.py

@ayesha1145 ayesha1145 force-pushed the feat/assignment-system branch from dd36a05 to 2bc2ffc Compare March 26, 2026 18:32
@ayesha1145
Copy link
Copy Markdown
Contributor Author

Addressed latest CodeRabbit feedback:

  • Fixed major: removed manual submissions delete — assignments.id CASCADE now handles it, eliminating the partial-failure window
  • Fixed minor: moved assignments section inside main content flow — no longer rendered after footer

@ayesha1145
Copy link
Copy Markdown
Contributor Author

@A1L13N This PR is ready for review — all feedback has been addressed ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant