feat: add session details tooltip to course detail calendar#955
Open
abdulateeb wants to merge 2 commits intoalphaonelabs:mainfrom
Open
feat: add session details tooltip to course detail calendar#955abdulateeb wants to merge 2 commits intoalphaonelabs:mainfrom
abdulateeb wants to merge 2 commits intoalphaonelabs:mainfrom
Conversation
Contributor
WalkthroughAdds per-day session tooltips to the course calendar: backend calendar payloads now include per-date session lists (title, start_time, end_time); template/JS render hoverable tooltips and ARIA attributes; tests updated to assert tooltip-ready session data in HTML and AJAX responses. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant TemplateJS as "Template JS"
participant Server
participant DB
Browser->>TemplateJS: Load course detail page
TemplateJS->>Server: Request calendar data (initial/AJAX)
Server->>DB: Query sessions for course (year/month)
DB-->>Server: Session rows
Server-->>TemplateJS: Calendar payload (weeks with per-day sessions)
TemplateJS-->>Browser: Render calendar days + attach tooltip HTML on days with sessions
Browser-->>Browser: Hover day -> show tooltip (session title + time)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/views.py (1)
3384-3397: 🧹 Nitpick | 🔵 TrivialAvoid rescanning month sessions for each day cell.
Line 3384 currently filters
month_sessionsper calendar day, which is avoidable work. Pre-group once by date (as done incourse_detail) and reuse.Refactor sketch
- # Get sessions for the current month - month_sessions = course.sessions.filter(start_time__year=year, start_time__month=month).order_by("start_time") + # Get sessions for the current month and group by date + month_sessions = course.sessions.filter(start_time__year=year, start_time__month=month).order_by("start_time") + session_map = {} + for s in month_sessions: + day_key = s.start_time.date() + if day_key not in session_map: + session_map[day_key] = [] + session_map[day_key].append( + { + "title": s.title, + "start_time": s.start_time.strftime("%I:%M %p"), + "end_time": s.end_time.strftime("%I:%M %p"), + } + ) @@ - sessions_on_day = [s for s in month_sessions if s.start_time.date() == date] + sessions_on_day = session_map.get(date, []) calendar_week.append( { "date": date.isoformat() if date else None, "has_session": bool(sessions_on_day), "is_today": date == today, - "sessions": [ - { - "title": s.title, - "start_time": s.start_time.strftime("%I:%M %p"), - "end_time": s.end_time.strftime("%I:%M %p"), - } - for s in sessions_on_day - ], + "sessions": sessions_on_day, } )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 3384 - 3397, The code repeatedly filters month_sessions for each day cell (sessions_on_day) causing unnecessary rescans; instead, pre-group month_sessions once into a dict keyed by date (as done in course_detail) — e.g., build a mapping like sessions_by_date = {s.start_time.date(): [s, ...] for s in month_sessions} before the calendar loop, then replace the per-day filter in the calendar_week construction to lookup sessions_by_date.get(date, []) and use that list to populate "has_session" and "sessions". Ensure you keep existing keys ("date", "has_session", "is_today", "sessions") and formatting of start_time/end_time when building the session dicts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/courses/detail.html`:
- Around line 480-493: Add proper ARIA so screen-readers can discover the
sessions: give the tooltip container div (the one with classes "absolute
bottom-full left-1/2 transform -translate-x-1/2 mb-2 hidden group-hover:block
z-50") a unique id and role="tooltip" and ensure it has aria-hidden toggled with
its visible state; on the trigger span (the element that users hover/click to
open the tooltip) add aria-describedby pointing to that tooltip id (generate the
id using the template context like day or forloop index to keep it unique).
Ensure the id is stable and that aria-hidden is true when the tooltip is hidden
and false when visible so assistive tech receives the correct state.
- Around line 1538-1554: The tooltip HTML is vulnerable because s.title,
s.start_time and s.end_time are interpolated directly into tooltipHtml before
assigning to grid.innerHTML; add a small HTML-escape helper (e.g., escapeHtml)
at the top of the script and use it when building items inside the
day.sessions.map callback (escape s.title, s.start_time, s.end_time) so the
generated tooltipHtml contains escaped text rather than raw user-provided HTML.
In `@web/tests/test_views.py`:
- Around line 585-629: Ruff is raising PT009 for the idiomatic Django TestCase
assertion methods used in test_calendar_view and test_calendar_ajax_endpoint; do
not modify these assertions—instead update the Ruff configuration to ignore
PT009 for test files by adding "PT009" to the ignore list for the test path
pattern (e.g. "**/tests/**") so project-wide linting allows
assertIn/assertEqual/assertTrue in tests; ensure the change is applied in your
repository's ruff config (pyproject.toml or .ruff.toml) and not inside the test
files themselves.
- Around line 584-597: Replace the brittle truthiness check
self.assertTrue(len(day["sessions"]) > 0) with an explicit assertion to improve
failure messages: use self.assertGreater(len(day["sessions"]), 0) to assert the
sessions list is non-empty; additionally, after assigning session_info =
day["sessions"][0], add self.assertIsNotNone(session_info) to clearly assert the
extracted session exists before checking its keys (keep the existing asserts for
"title", "start_time", "end_time" and the equality against
self.future_session.title).
- Around line 599-629: In test_calendar_ajax_endpoint, strengthen assertions and
coverage: replace assertTrue(len(day["sessions"]) > 0) with
self.assertGreater(len(day["sessions"]), 0) and explicitly validate that
session_info["start_time"] and session_info["end_time"] are ISO 8601 strings
(e.g., parseable with datetime.fromisoformat or match
r'^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}'), and add two additional subtests that
call the same endpoint authenticated as a teacher and as a student (use the
existing test fixtures like self.teacher_user / self.student_user and
client.force_login or client.login) asserting status_code == 200 and that the
session appears in response.json()["calendar_weeks"] with correctly formatted
times.
In `@web/views.py`:
- Line 853: The out-of-month day cell appended in course_detail currently omits
the is_today key, causing schema mismatch with AJAX calendar payload; update the
calendar_week.append call in the course_detail view (the line creating {"date":
None, "in_month": False, "has_session": False, "sessions": []}) to include
"is_today": False so all day-cell dictionaries always have the same keys.
---
Outside diff comments:
In `@web/views.py`:
- Around line 3384-3397: The code repeatedly filters month_sessions for each day
cell (sessions_on_day) causing unnecessary rescans; instead, pre-group
month_sessions once into a dict keyed by date (as done in course_detail) — e.g.,
build a mapping like sessions_by_date = {s.start_time.date(): [s, ...] for s in
month_sessions} before the calendar loop, then replace the per-day filter in the
calendar_week construction to lookup sessions_by_date.get(date, []) and use that
list to populate "has_session" and "sessions". Ensure you keep existing keys
("date", "has_session", "is_today", "sessions") and formatting of
start_time/end_time when building the session dicts.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/tests/test_views.py (1)
599-629: 🧹 Nitpick | 🔵 TrivialAJAX endpoint test looks solid; minor hardening opportunity.
The test correctly validates the JSON structure including
calendar_weeks,current_month, and per-day session details. One observation:The test doesn't assert anything about the
start_time/end_timevalues — only their presence. If the serialization format changes (e.g., from"05:39 AM"to ISO 8601), this test would still pass silently. Consider adding a lightweight format or non-empty check.♻️ Optional: assert times are non-empty strings
self.assertIn("start_time", session_info) self.assertIn("end_time", session_info) + self.assertTrue(session_info["start_time"], "start_time should be non-empty") + self.assertTrue(session_info["end_time"], "end_time should be non-empty")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/tests/test_views.py` around lines 599 - 629, Update test_calendar_ajax_endpoint to assert that the serialized times are meaningful: after locating session_info (from data["calendar_weeks"] in test_calendar_ajax_endpoint), add checks that session_info["start_time"] and session_info["end_time"] are non-empty strings and optionally match a light format (e.g., contain digits or a ":" or are valid ISO-like substrings) instead of only asserting their presence; use the existing future_session reference to compare or validate consistency if you prefer a stricter check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/courses/detail.html`:
- Around line 1545-1576: The tooltip only appears on hover; update the trigger
span (the element built where class uses day.has_session and ariaAttr is
applied) to include tabindex="0" when day.has_session so it is
keyboard-focusable, and modify the tooltip container markup (where tooltipHtml
sets class="absolute ... hidden group-hover:block ...") to also include
group-focus-within:block so focus opens the tooltip; ensure these changes are
applied both in the JS-built tooltipHtml and the server-rendered template
referenced by ariaAttr/day.has_session.
- Around line 481-496: The tooltip currently uses a fixed "bottom-full" position
which can push it above the visible calendar for first-row dates; modify the
tooltip container (id "tooltip-{{ day.date|date:'Y-m-d' }}") to use a
conditional class that flips it when the date is in the first calendar row
(e.g., {% if day.is_first_row %}use "top-full mt-2" and adjust the arrow to
point up{% else %}keep "bottom-full" and current arrow{% endif %}). To do this
either add a boolean (is_first_row or week_index == 0) to the day objects in the
view context or compute it in the template, then apply the conditional class and
corresponding arrow markup adjustments inside the tooltip block so first-row
tooltips render below the date.
In `@web/views.py`:
- Around line 830-833: month_sessions is filtered from sessions which was
previously reordered (future-first), causing tooltip entries for the same date
to be out of chronological order; fix by ensuring month_sessions is ordered by
session start_time ascending (either filter from an original
chronologically-sorted source or sort month_sessions by the session.start_time
key) so tooltip entries appear in chronological order.
---
Duplicate comments:
In `@web/tests/test_views.py`:
- Around line 599-629: Update test_calendar_ajax_endpoint to assert that the
serialized times are meaningful: after locating session_info (from
data["calendar_weeks"] in test_calendar_ajax_endpoint), add checks that
session_info["start_time"] and session_info["end_time"] are non-empty strings
and optionally match a light format (e.g., contain digits or a ":" or are valid
ISO-like substrings) instead of only asserting their presence; use the existing
future_session reference to compare or validate consistency if you prefer a
stricter check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
Fixes #954
Checklist
Problem
The session calendar on the course detail page highlights dates with sessions (teal text + dot indicator), but hovering over those dates provides no information about which session is scheduled. Users must scroll to the sessions list to find out.
Solution
Added hover tooltips to calendar dates showing session title and time range.
Screenshots
Before:
After:
Summary by CodeRabbit