Implement session waiting room feature using existing WaitingRoom model#650
Conversation
Co-authored-by: A1L13N <193832434+A1L13N@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
A1L13N
left a comment
A1L13N
left a comment
There was a problem hiding this comment.
Please update this PR:
✅ Ensure all review comments are addressed
🔄 Resolve any merge conflicts
🧭 Verify that database migrations are correct and up to date
Once everything is done, please push the updated commits so we can proceed with the review.
There was a problem hiding this comment.
Pull Request Overview
This PR implements a session waiting room feature that allows students to express interest in upcoming sessions after a current session has ended. The feature automatically detects ended sessions and provides UI for students to join waiting rooms for the next scheduled session.
Key changes include:
- New
SessionWaitingRoommodel with course-specific waiting rooms - Enhanced session detail view to show waiting room functionality for ended sessions
- Join/leave waiting room endpoints with proper authentication
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/models.py | Added SessionWaitingRoom model with participant management and course relationship |
| web/views.py | Enhanced session detail view with waiting room context and added join/leave views |
| web/urls.py | Added URL patterns for session waiting room join/leave actions |
| web/templates/web/study/session_detail.html | Added UI for waiting room functionality when sessions have ended |
| web/tests/test_session_waiting_room.py | Comprehensive test suite covering model functionality and view behavior |
| web/migrations/0062_sessionwaitingroom.py | Database migration to create SessionWaitingRoom table |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def participant_count(self): | ||
| """Return the number of participants in the waiting room.""" | ||
| return self.participants.count() | ||
|
|
||
| def topic_list(self): | ||
| """Return the list of topics as a list.""" | ||
| return [topic.strip() for topic in self.topics.split(",") if topic.strip()] | ||
|
|
||
| def mark_as_fulfilled(self, course=None): | ||
| """Mark the waiting room as fulfilled and notify participants.""" | ||
| self.status = "fulfilled" | ||
| self.save() | ||
|
|
||
| if course: | ||
| from .notifications import notify_waiting_room_fulfilled | ||
|
|
||
| notify_waiting_room_fulfilled(self, course) |
There was a problem hiding this comment.
These methods appear to belong to the WaitingRoom class but are incorrectly placed after the SessionWaitingRoom class definition. They should be moved back to their proper location within the WaitingRoom class.
| # Get the next upcoming session for this course | ||
| next_session = session.course.sessions.filter(start_time__gt=timezone.now()).order_by('start_time').first() |
There was a problem hiding this comment.
This query is executed for every authenticated user viewing a session detail, even when the session hasn't ended. Consider moving this query inside the condition that checks if the session has ended to avoid unnecessary database queries.
| # Get the next upcoming session for this course | |
| next_session = session.course.sessions.filter(start_time__gt=timezone.now()).order_by('start_time').first() | |
| # Only get the next session if the current session has ended | |
| next_session = None | |
| if timezone.now() > session.end_time: | |
| next_session = session.course.sessions.filter(start_time__gt=timezone.now()).order_by('start_time').first() |
| session_waiting_room.participants.add(request.user) | ||
| next_session = session_waiting_room.get_next_session() | ||
| if next_session: | ||
| messages.success(request, f"You have joined the waiting room for the next session of {course.title}. Next session is on {next_session.start_time.strftime('%B %d, %Y at %I:%M %p')}.") |
There was a problem hiding this comment.
[nitpick] The date formatting string should be consistent with other parts of the application. Consider using Django's date formatting utilities or defining a constant for the date format to ensure consistency across the codebase.
| updated_at = models.DateTimeField(auto_now=True) | ||
|
|
||
| class Meta: | ||
| unique_together = ["course"] # Only one waiting room per course |
There was a problem hiding this comment.
The unique_together constraint should use a tuple instead of a list for single field constraints. Consider using unique_together = ('course',) or better yet, add unique=True to the course field directly.
| unique_together = ["course"] # Only one waiting room per course | |
| unique_together = ('course',) # Only one waiting room per course |
|
@copilot Fix the tests / lints |
Co-authored-by: A1L13N <193832434+A1L13N@users.noreply.github.com>
Fixed all linting issues in commit 7b5fe29. All code has been formatted with black, isort, flake8, and djlint. All 216 tests pass successfully. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot fix lint for reformatted web/migrations/0062_sessionwaitingroom.py |
Co-authored-by: A1L13N <193832434+A1L13N@users.noreply.github.com>
Fixed linting for migration file in commit {commit_hash}. The file has been formatted with black to fix line length violations (lines 20, 23, 24 were over 120 characters). |
|
@copilot refactor this to use existing waiting room model |
Co-authored-by: A1L13N <193832434+A1L13N@users.noreply.github.com>
Refactored to use existing WaitingRoom model in commit {commit_hash}. Changes:
All 7 tests pass successfully. |
Implements a session waiting room feature that allows students to express interest in upcoming sessions of a course after a session has ended. The implementation extends the existing
WaitingRoommodel rather than creating a duplicate model.Changes Made
Model Refactor
WaitingRoommodel to support both use cases:coursefield to link to existing coursestitle,subject,topics,creatorfields optional (only required for new course requests)get_next_session()method to retrieve the next upcoming sessionclose_waiting_room()method for managing waiting room statusViews & Templates
join_session_waiting_roomview for users to join the waiting roomleave_session_waiting_roomview for users to leave the waiting roomTesting
Database Migration
0062_update_waitingroom_for_sessions.pyupdates theWaitingRoommodel with new fields and constraintsFeatures
end_time < now)Testing
This implementation provides a seamless way for students to express continued interest in a course after a session has concluded, helping instructors gauge demand for future sessions while reusing existing infrastructure.
Fixes #645.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.