feat: enhance dashboard with student progress tracking and session attendance#21
feat: enhance dashboard with student progress tracking and session attendance#21ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
Conversation
WalkthroughAdds POST /api/attendance to record per-session attendance; extends GET /api/dashboard to return per-activity Changes
Sequence DiagramsequenceDiagram
participant User as User (Client)
participant UI as Dashboard UI
participant API as Worker (Server)
participant DB as Database
User->>UI: mark attendance (session_id, status)
UI->>API: POST /api/attendance {session_id, status}
API->>API: validate auth & inputs
API->>DB: SELECT session, activity, enrollment
DB-->>API: session + enrollment result
API->>DB: INSERT INTO session_attendance ON CONFLICT UPDATE
DB-->>API: upsert confirmation
API-->>UI: {session_id, status}
UI->>API: GET /api/dashboard
API->>DB: query joined activities, counts (total_sessions, attended_sessions), hosted activities, batched tags
DB-->>API: activities + stats + tags
API-->>UI: {stats, joined_activities[], hosted_activities[]}
UI->>User: render stat cards and per-activity progress
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/dashboard.html`:
- Around line 154-158: The progress bar string assigned to progressBar lacks
ARIA attributes; update the generated element for the progress bar (the div
containing class "bg-indigo-500 ...") to include role="progressbar",
aria-valuemin="0", aria-valuemax equal to a.total_sessions or 100 depending on
your model, aria-valuenow set to the numeric value (use a.attended_sessions or
pct), and an aria-label or aria-valuetext that describes the progress (e.g.,
“Progress: X of Y sessions (Z%)”) so screen readers can perceive the same info
as the visual meter; locate the variables pct, a.progress_pct,
a.attended_sessions and a.total_sessions and inject these attributes into the
progress bar div generation.
In `@src/worker.py`:
- Around line 13-14: Replace the EN DASH characters in the route documentation
string with normal hyphen-minus characters so the paths are copy-paste safe;
locate the module-level docstring or the string containing "GET /api/dashboard
– personal dashboard with progress stats" and "POST /api/attendance –
mark session attendance [auth]" and swap each "–" to "-"
(hyphen-minus) in that string.
- Around line 914-925: The loop builds hosted entries using dict-style row
access (r["id"], r["title"], etc.) which is inconsistent with the rest of the
file (use attribute access like row.id). Update the loop that iterates over
res.results to use attribute access (e.g., r.id, r.title, r.type, r.format,
r.schedule_type, r.participant_count, r.session_count, r.created_at) and also
map tag rows from t_res to t.name instead of t["name"]; keep the same
prepare/bind logic and the hosted append shape unchanged.
- Around line 1244-1255: The current check-then-insert/update using
env.DB.prepare with a SELECT then conditional UPDATE/INSERT on
session_attendance (session_id,user_id) is racy and lacks error handling;
replace it with an atomic upsert using SQLite's INSERT ... ON CONFLICT
(session_id, user_id) DO UPDATE to set status (or use a single SQL statement
executed via env.DB.prepare) and remove the separate SELECT branch, and if you
keep the two-step flow then wrap the INSERT in a try/except similar to api_join
to catch UNIQUE constraint errors, log the failure, and return a controlled
response via ok/error; target the block around the functions/variables
env.DB.prepare, session_attendance, new_id(), status, and ok().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 33b20b7d-3bcb-4ca3-8300-f7884bfbd7b3
📒 Files selected for processing (2)
public/dashboard.htmlsrc/worker.py
02be33c to
85d9421
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worker.py`:
- Around line 915-917: The per-activity tag SELECT (using env.DB.prepare(...)
and binding r["id"] into t_res) must be replaced with a single batched query:
collect all activity IDs returned earlier, run one SELECT joining tags and
activity_tags with WHERE activity_id IN (...), then group results into a mapping
of activity_id -> list of tags in Python and use that map when building the
hosted and joined lists (the current per-row logic that reads t_res should be
switched to lookup in this map); also apply the same batching change for the
similar lookup around the code referenced at lines 943-945.
- Around line 1224-1228: The code currently defaults status = body.get("status",
"attended") which can silently mark missing statuses as attended; change this to
require an explicit status by using status = body.get("status") (no default) and
add a check if not status: return err("status is required"), then keep the
existing validation if status not in ("registered","attended","missed") to
return err("status must be registered, attended, or missed"); reference the
variables/status check and err(...) in the same block (status, session_id,
body.get, err) when making the change.
- Around line 1238-1242: The current gate uses is_host to allow hosts to bypass
enrollment (is_host from env.DB.prepare(...).bind(...).first()) so hosts can
write attendance without an enrollment row; change the logic to require an
enrollment row (enr) regardless of is_host by removing is_host from the
allowlist and returning err("You must be enrolled in this activity", 403) when
enr is falsy; keep the activity existence check if needed but do not let is_host
bypass the enrollment requirement in this handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 170949b9-900e-4003-b44f-c6467b6a1a0b
📒 Files selected for processing (2)
public/dashboard.htmlsrc/worker.py
85d9421 to
117be7c
Compare
|
Addressed latest CodeRabbit feedback:
|
|
@A1L13N This PR is ready for review — all feedback has been addressed ✅ |
|
@copilot fix the conflicts |
117be7c to
93e65df
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/worker.py (2)
13-14:⚠️ Potential issue | 🟡 MinorMinor: Replace EN DASH with HYPHEN-MINUS in route documentation.
The docstring uses EN DASH (
–) instead of the standard HYPHEN-MINUS (-). While this doesn't affect runtime behavior, it can cause copy-paste issues if someone tries to use these paths directly.📝 Suggested fix
- GET /api/dashboard – personal dashboard with progress stats - POST /api/attendance – mark session attendance [auth] + GET /api/dashboard - personal dashboard with progress stats + POST /api/attendance - mark session attendance [auth]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` around lines 13 - 14, The route documentation in the docstring uses EN DASH characters (–) after the paths (e.g. the lines with "GET /api/dashboard – personal dashboard with progress stats" and "POST /api/attendance – mark session attendance [auth]"); update those separators to the standard ASCII hyphen-minus (-) so the lines become "GET /api/dashboard - personal dashboard..." and "POST /api/attendance - mark session attendance..." to avoid copy-paste issues.
1063-1063:⚠️ Potential issue | 🔴 CriticalCritical: Using deprecated
decrypt()instead ofawait decrypt_aes().This line calls the synchronous
decrypt()function, which has been deprecated and will raise aRuntimeErrorat runtime (see lines 203-205). This will cause the entire dashboard endpoint to fail for any user with joined activities.🐛 Proposed fix
- "host_name": decrypt(r["host_name_enc"] or "", enc), + "host_name": await decrypt_aes(r["host_name_enc"] or "", enc),As per coding guidelines: "Ensure proper error handling and logging."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` at line 1063, Replace the deprecated synchronous decrypt() call with the async decrypt_aes() and await it where the host_name is built (replace "host_name": decrypt(r["host_name_enc"] or "", enc) with an awaited call to decrypt_aes); ensure the enclosing function is async or that you use await within an async map/gather, and wrap the await in try/except to catch RuntimeError (or any decryption error) and log the error via the existing logger before falling back to a safe value (e.g., empty string) so runtime failures for joined activities don’t crash the endpoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worker.py`:
- Around line 1351-1358: The capture_exception call inside the exception handler
for the upsert in api_mark_attendance is missing request and environment
context; update the exception handling around
env.DB.prepare(...).bind(...).run() so that capture_exception(exc, req=req,
env=env, where="api_mark_attendance") (or the project's canonical param names)
is invoked before returning the error, ensuring req and env are passed alongside
exc and where to capture full context for debugging.
---
Duplicate comments:
In `@src/worker.py`:
- Around line 13-14: The route documentation in the docstring uses EN DASH
characters (–) after the paths (e.g. the lines with "GET /api/dashboard
– personal dashboard with progress stats" and "POST /api/attendance –
mark session attendance [auth]"); update those separators to the
standard ASCII hyphen-minus (-) so the lines become "GET /api/dashboard
- personal dashboard..." and "POST /api/attendance - mark session
attendance..." to avoid copy-paste issues.
- Line 1063: Replace the deprecated synchronous decrypt() call with the async
decrypt_aes() and await it where the host_name is built (replace "host_name":
decrypt(r["host_name_enc"] or "", enc) with an awaited call to decrypt_aes);
ensure the enclosing function is async or that you use await within an async
map/gather, and wrap the await in try/except to catch RuntimeError (or any
decryption error) and log the error via the existing logger before falling back
to a safe value (e.g., empty string) so runtime failures for joined activities
don’t crash the endpoint.
🪄 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: 0eaf437a-cc34-4c85-9dfb-efed9fa8ebfb
📒 Files selected for processing (2)
public/dashboard.htmlsrc/worker.py
93e65df to
c0fb4a9
Compare
|
Fixed nitpick: added req and env parameters to capture_exception() in api_mark_attendance for richer error logging context in production. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worker.py`:
- Around line 999-1078: Add tests that cover the new /api/dashboard aggregation
and the enrollment-gated attendance upsert flows: create fixtures for activities
with zero sessions and with >0 sessions, users who are enrolled (enr_status
values including default "registered" and "active") and non-enrolled users, and
exercise re-marking the same session twice. For the dashboard endpoint assert
hosted and joined payload shapes (hosted, joined lists), joined[*].progress_pct,
joined[*].attended_sessions, stats keys (total_joined, completed, in_progress,
total_sessions_attended, hosted_count) and that zero-session activities yield
progress_pct 0 and don’t inflate in_progress; for attendance upserts simulate
attendance calls for an enrolled vs non-enrolled user and re-marking the same
session to ensure attendance increments only when allowed and idempotency holds.
Locate tests to cover the code paths populating joined, hosted, stats and the
enrollment-gated attendance upsert logic referenced in the PR.
- Around line 1059-1068: The code is calling the deprecated sync shim
decrypt(...) which raises RuntimeError; replace that call with the async decrypt
helper (e.g., await decrypt_async(...) or await decrypt(... ) if the async
function is named decrypt) when building the "host_name" field so the operation
is awaited and non-blocking; update the surrounding function (the
generator/handler that builds joined) to be async or perform an asyncio.gather
over multiple decrypt calls so the list comprehension/loop that produces joined
can await the async helper for r["host_name_enc"] and pass the enc parameter
correctly.
🪄 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: a07fd1f8-62ea-499b-a9a2-c38912630c98
📒 Files selected for processing (2)
public/dashboard.htmlsrc/worker.py
| # Hosted activities | ||
| res = await env.DB.prepare( | ||
| "SELECT a.id,a.title,a.type,a.format,a.schedule_type,a.created_at," | ||
| "(SELECT COUNT(*) FROM enrollments WHERE activity_id=a.id AND status='active')" | ||
| " AS participant_count," | ||
| "SELECT a.id, a.title, a.type, a.format, a.schedule_type, a.created_at," | ||
| "(SELECT COUNT(*) FROM enrollments WHERE activity_id=a.id AND status='active') AS participant_count," | ||
| "(SELECT COUNT(*) FROM sessions WHERE activity_id=a.id) AS session_count" | ||
| " FROM activities a WHERE a.host_id=? ORDER BY a.created_at DESC" | ||
| ).bind(user["id"]).all() | ||
| hosted_rows = res.results or [] | ||
| hosted_ids = [r["id"] for r in hosted_rows] | ||
| hosted_tags = {} | ||
| if hosted_ids: | ||
| placeholders = ",".join("?" * len(hosted_ids)) | ||
| tag_res = await env.DB.prepare( | ||
| f"SELECT at2.activity_id, t.name FROM tags t JOIN activity_tags at2 ON at2.tag_id=t.id" | ||
| f" WHERE at2.activity_id IN ({placeholders})" | ||
| ).bind(*hosted_ids).all() | ||
| for tr in (tag_res.results or []): | ||
| hosted_tags.setdefault(tr["activity_id"], []).append(tr["name"]) | ||
| hosted = [ | ||
| { | ||
| "id": r["id"], "title": r["title"], "type": r["type"], | ||
| "format": r["format"], "schedule_type": r["schedule_type"], | ||
| "participant_count": r["participant_count"] or 0, | ||
| "session_count": r["session_count"] or 0, | ||
| "tags": hosted_tags.get(r["id"], []), | ||
| "created_at": r["created_at"], | ||
| } | ||
| for r in hosted_rows | ||
| ] | ||
|
|
||
| hosted = [] | ||
| for r in res.results or []: | ||
| t_res = await env.DB.prepare( | ||
| "SELECT t.name FROM tags t JOIN activity_tags at2 ON at2.tag_id=t.id" | ||
| " WHERE at2.activity_id=?" | ||
| ).bind(r.id).all() | ||
| hosted.append({ | ||
| "id": r.id, | ||
| "title": r.title, | ||
| "type": r.type, | ||
| "format": r.format, | ||
| "schedule_type": r.schedule_type, | ||
| "participant_count": r.participant_count, | ||
| "session_count": r.session_count, | ||
| "tags": [t.name for t in (t_res.results or [])], | ||
| "created_at": r.created_at, | ||
| }) | ||
|
|
||
| # Joined activities with progress | ||
| res2 = await env.DB.prepare( | ||
| "SELECT a.id,a.title,a.type,a.format,a.schedule_type," | ||
| "e.role AS enr_role,e.status AS enr_status,e.created_at AS joined_at," | ||
| "u.name AS host_name_enc" | ||
| "SELECT a.id, a.title, a.type, a.format, a.schedule_type," | ||
| " e.role AS enr_role, e.status AS enr_status, e.created_at AS joined_at," | ||
| " u.name AS host_name_enc," | ||
| " (SELECT COUNT(*) FROM sessions WHERE activity_id=a.id) AS total_sessions," | ||
| " (SELECT COUNT(*) FROM session_attendance sa" | ||
| " JOIN sessions s ON s.id=sa.session_id" | ||
| " WHERE s.activity_id=a.id AND sa.user_id=? AND sa.status='attended') AS attended_sessions" | ||
| " FROM enrollments e" | ||
| " JOIN activities a ON e.activity_id=a.id" | ||
| " JOIN users u ON a.host_id=u.id" | ||
| " WHERE e.user_id=? ORDER BY e.created_at DESC" | ||
| ).bind(user["id"]).all() | ||
|
|
||
| ).bind(user["id"], user["id"]).all() | ||
| joined_rows = res2.results or [] | ||
| joined_ids = [r["id"] for r in joined_rows] | ||
| joined_tags = {} | ||
| if joined_ids: | ||
| placeholders2 = ",".join("?" * len(joined_ids)) | ||
| tag_res2 = await env.DB.prepare( | ||
| f"SELECT at2.activity_id, t.name FROM tags t JOIN activity_tags at2 ON at2.tag_id=t.id" | ||
| f" WHERE at2.activity_id IN ({placeholders2})" | ||
| ).bind(*joined_ids).all() | ||
| for tr in (tag_res2.results or []): | ||
| joined_tags.setdefault(tr["activity_id"], []).append(tr["name"]) | ||
| joined = [] | ||
| for r in res2.results or []: | ||
| t_res = await env.DB.prepare( | ||
| "SELECT t.name FROM tags t JOIN activity_tags at2 ON at2.tag_id=t.id" | ||
| " WHERE at2.activity_id=?" | ||
| ).bind(r.id).all() | ||
| for r in joined_rows: | ||
| total = r["total_sessions"] or 0 | ||
| attended = r["attended_sessions"] or 0 | ||
| progress = round((attended / total) * 100) if total > 0 else 0 | ||
| joined.append({ | ||
| "id": r.id, | ||
| "title": r.title, | ||
| "type": r.type, | ||
| "format": r.format, | ||
| "schedule_type": r.schedule_type, | ||
| "enr_role": r.enr_role, | ||
| "enr_status": r.enr_status, | ||
| "host_name": await decrypt_aes(r.host_name_enc or "", enc), | ||
| "tags": [t.name for t in (t_res.results or [])], | ||
| "joined_at": r.joined_at, | ||
| "id": r["id"], "title": r["title"], "type": r["type"], | ||
| "format": r["format"], "schedule_type": r["schedule_type"], | ||
| "enr_role": r["enr_role"], "enr_status": r["enr_status"], | ||
| "host_name": decrypt(r["host_name_enc"] or "", enc), | ||
| "tags": joined_tags.get(r["id"], []), | ||
| "joined_at": r["joined_at"], | ||
| "total_sessions": total, | ||
| "attended_sessions": attended, | ||
| "progress_pct": progress, | ||
| }) | ||
|
|
||
| return json_resp({"user": user, "hosted_activities": hosted, "joined_activities": joined}) | ||
|
|
||
| stats = { | ||
| "total_joined": len(joined), | ||
| "completed": sum(1 for a in joined if a["enr_status"] == "completed"), | ||
| "in_progress": sum(1 for a in joined if a["enr_status"] == "active" and a["total_sessions"] > 0), | ||
| "total_sessions_attended": sum(a["attended_sessions"] for a in joined), | ||
| "hosted_count": len(hosted), | ||
| } | ||
| return json_resp({"user": user, "hosted_activities": hosted, "joined_activities": joined, "stats": stats}) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add regression coverage for the new dashboard and attendance flows.
This PR changes /api/dashboard aggregation and adds enrollment-gated attendance upserts, but there’s no test coverage in the diff for zero-session activities, non-enrolled users, default "registered", or re-marking the same session. A small test matrix here would catch regressions like the Line 1063 failure quickly.
As per coding guidelines: "Verify tests cover the key logic paths."
Also applies to: 1325-1359
🧰 Tools
🪛 Ruff (0.15.7)
[error] 1012-1013: Possible SQL injection vector through string-based query construction
(S608)
[error] 1049-1050: Possible SQL injection vector through string-based query construction
(S608)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/worker.py` around lines 999 - 1078, Add tests that cover the new
/api/dashboard aggregation and the enrollment-gated attendance upsert flows:
create fixtures for activities with zero sessions and with >0 sessions, users
who are enrolled (enr_status values including default "registered" and "active")
and non-enrolled users, and exercise re-marking the same session twice. For the
dashboard endpoint assert hosted and joined payload shapes (hosted, joined
lists), joined[*].progress_pct, joined[*].attended_sessions, stats keys
(total_joined, completed, in_progress, total_sessions_attended, hosted_count)
and that zero-session activities yield progress_pct 0 and don’t inflate
in_progress; for attendance upserts simulate attendance calls for an enrolled vs
non-enrolled user and re-marking the same session to ensure attendance
increments only when allowed and idempotency holds. Locate tests to cover the
code paths populating joined, hosted, stats and the enrollment-gated attendance
upsert logic referenced in the PR.
| joined.append({ | ||
| "id": r.id, | ||
| "title": r.title, | ||
| "type": r.type, | ||
| "format": r.format, | ||
| "schedule_type": r.schedule_type, | ||
| "enr_role": r.enr_role, | ||
| "enr_status": r.enr_status, | ||
| "host_name": await decrypt_aes(r.host_name_enc or "", enc), | ||
| "tags": [t.name for t in (t_res.results or [])], | ||
| "joined_at": r.joined_at, | ||
| "id": r["id"], "title": r["title"], "type": r["type"], | ||
| "format": r["format"], "schedule_type": r["schedule_type"], | ||
| "enr_role": r["enr_role"], "enr_status": r["enr_status"], | ||
| "host_name": decrypt(r["host_name_enc"] or "", enc), | ||
| "tags": joined_tags.get(r["id"], []), | ||
| "joined_at": r["joined_at"], | ||
| "total_sessions": total, | ||
| "attended_sessions": attended, | ||
| "progress_pct": progress, |
There was a problem hiding this comment.
Switch host_name back to the async decrypt helper.
Line 1063 calls the deprecated sync decrypt() shim, which raises RuntimeError by design. Any user with at least one joined activity will get a 500 from GET /api/dashboard.
🐛 Proposed fix
joined.append({
"id": r["id"], "title": r["title"], "type": r["type"],
"format": r["format"], "schedule_type": r["schedule_type"],
"enr_role": r["enr_role"], "enr_status": r["enr_status"],
- "host_name": decrypt(r["host_name_enc"] or "", enc),
+ "host_name": await decrypt_aes(r["host_name_enc"] or "", enc),
"tags": joined_tags.get(r["id"], []),
"joined_at": r["joined_at"],
"total_sessions": total,
"attended_sessions": attended,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/worker.py` around lines 1059 - 1068, The code is calling the deprecated
sync shim decrypt(...) which raises RuntimeError; replace that call with the
async decrypt helper (e.g., await decrypt_async(...) or await decrypt(... ) if
the async function is named decrypt) when building the "host_name" field so the
operation is awaited and non-blocking; update the surrounding function (the
generator/handler that builds joined) to be async or perform an asyncio.gather
over multiple decrypt calls so the list comprehension/loop that produces joined
can await the async helper for r["host_name_enc"] and pass the enc parameter
correctly.
Adds student progress tracking to the dashboard and a new attendance API endpoint.
Changes:
Features:
Purpose
Enhance the dashboard with per-activity student progress tracking and add session attendance management so enrolled users can mark and update attendance. Provide server-driven aggregate stats to simplify client logic and improve consistency.
Key Modifications
Backend (src/worker.py)
Frontend (public/dashboard.html)
Impact on Functionality & UX
Notes for Review