diff --git a/ACEM_DESIGN_66.md b/ACEM_DESIGN_66.md new file mode 100644 index 0000000..4cfb24b --- /dev/null +++ b/ACEM_DESIGN_66.md @@ -0,0 +1,351 @@ +# Design Document: Allow Students to See Course They Are Not Enrolled In + +## Issue Summary + +**Problem**: When a student tries to view a course they are not enrolled in, they receive a "You do not have access to this course" error (HTTP 403). + +**Desired Behavior**: Students should be able to view the course page for any active course, seeing: +- Course name and code +- Course description +- Instructors +- An "Enroll Now" button (if not enrolled) + +## Feedback from User + +The `user_roles` used in the codebase should not be strings but enums. A new enum for course roles should be created. + +## Current Behavior + +In `src/courses/views.py`, the `CourseDetailView.get()` method (lines 339-383) checks user roles and returns 403 if the user has no role (teacher, enrolled student, or TA) for the course: + +```python +if not user_roles: + return HttpResponseForbidden("You do not have access to this course.") +``` + +## Proposed Changes + +### 1. Create Course Role Enum + +**File**: `src/courses/enums.py` (new file) + +```python +from enum import StrEnum + + +class CourseRole(StrEnum): + """Enumeration of possible user roles within a course context.""" + + TEACHER = "teacher" + STUDENT = "student" + TEACHER_ASSISTANT = "teacher_assistant" +``` + +### 2. Update CourseDetailData Dataclass + +**File**: `src/courses/views.py` +**Location**: Lines 310-323 + +**Changes**: +- Add `instructors` field +- Add `is_enrolled` flag for template to conditionally show enrollment status +- Update `user_roles` type to use `CourseRole` enum + +```python +from .enums import CourseRole # Add import + +@dataclass +class CourseDetailData: + """Data structure for the course detail view.""" + + course_id: UUID + course_name: str + course_code: str + course_description: str + homeworks: list[HomeworkItem] + enrolled_students: list[EnrolledStudentItem] | None + teacher_assistants: list[TAItem] | None + user_roles: list[CourseRole] # Changed from list[str] + instructors: list[InstructorItem] # NEW: List of instructors + is_enrolled: bool # NEW: True if user is enrolled student +``` + +### 3. Modify CourseDetailView.get() Method + +**File**: `src/courses/views.py` +**Location**: Lines 339-383 + +**Change**: Instead of returning 403 when a student has no role, allow access with limited data. + +```python +def get(self, request: HttpRequest, course_id: UUID) -> HttpResponse: + """Handle GET requests to display course detail.""" + course = get_object_or_404(Course, id=course_id) + + teacher_profile = getattr(request.user, "teacher_profile", None) + student_profile = getattr(request.user, "student_profile", None) + teacher_assistant_profile = getattr( + request.user, "teacher_assistant_profile", None + ) + + user_roles: list[CourseRole] = [] # Changed type + + if ( + teacher_profile + and CourseTeacher.objects.filter( + course=course, teacher=teacher_profile + ).exists() + ): + user_roles.append(CourseRole.TEACHER) + + if student_profile and course.is_student_enrolled(student_profile): + user_roles.append(CourseRole.STUDENT) + + if teacher_assistant_profile and course.is_teacher_assistant( + teacher_assistant_profile + ): + user_roles.append(CourseRole.TEACHER_ASSISTANT) + + # Get the appropriate data based on user roles + data = self._get_view_data( + course, + user_roles, + teacher_profile, + student_profile, + teacher_assistant_profile, + ) + + return render(request, "courses/detail.html", {"data": data}) +``` + +### 4. Modify _get_view_data() Method + +**File**: `src/courses/views.py` +**Location**: Lines 385-472 + +**Changes**: +- Update signature to use `CourseRole` +- Add `instructors` and `is_enrolled` to returned data +- Remove the logic that restricts homework visibility based on enrollment + +```python +def _get_view_data( + self, + course: Course, + user_roles: list[CourseRole], + teacher_profile=None, + student_profile=None, + teacher_assistant_profile=None, +) -> CourseDetailData: + # ... existing homework retrieval logic (lines 406-420) ... + + # Get instructors + instructors = [] + for ct in CourseTeacher.objects.filter(course=course).select_related("teacher__user"): + instructors.append( + InstructorItem( + first_name=ct.teacher.user.first_name, + last_name=ct.teacher.user.last_name, + ) + ) + + # Determine if user is enrolled as student + is_enrolled = CourseRole.STUDENT in user_roles + + # Get enrolled students if user is a teacher or TA + enrolled_students = None + if CourseRole.TEACHER in user_roles or CourseRole.TEACHER_ASSISTANT in user_roles: + enrollments = ( + CourseEnrollment.objects.filter(course=course, is_active=True) + .select_related("student__user") + .order_by("-enrolled_at") + ) + enrolled_students = [] + for enrollment in enrollments: + student = enrollment.student + enrolled_students.append( + EnrolledStudentItem( + id=student.id, + username=student.user.username, + email=student.user.email, + enrolled_at=enrollment.enrolled_at.strftime("%B %d, %Y"), + ) + ) + + # Get teacher assistants if user is a teacher or TA + teacher_assistants = None + if CourseRole.TEACHER in user_roles or CourseRole.TEACHER_ASSISTANT in user_roles: + tas = ( + CourseTeacherAssistant.objects.filter(course=course) + .select_related("teacher_assistant__user") + .order_by("-assigned_at") + ) + teacher_assistants = [] + for ta in tas: + teacher_assistants.append( + TAItem( + id=ta.teacher_assistant.id, + username=ta.teacher_assistant.user.username, + email=ta.teacher_assistant.user.email, + assigned_at=ta.assigned_at.strftime("%B %d, %Y"), + ) + ) + + return CourseDetailData( + course_id=course.id, + course_name=course.name, + course_code=course.code, + course_description=course.description, + homeworks=homeworks, # Show homeworks to everyone + enrolled_students=enrolled_students, + teacher_assistants=teacher_assistants, + user_roles=user_roles, + instructors=instructors, + is_enrolled=is_enrolled, + ) +``` + +### 5. Modify CourseEnrollView.post() Method + +**File**: `src/courses/views.py` +**Location**: Line 225 + +**Change**: After successful enrollment, redirect to the course detail page instead of the course list. + +```python +# Redirect back to course detail +return redirect("courses:detail", course_id=course.id) +``` + +### 6. Update CourseListView to Use CourseRole Enum + +**File**: `src/courses/views.py` + +Update `CourseItem` and `CourseListData` to use `CourseRole`: + +```python +from .enums import CourseRole + +@dataclass +class CourseItem: + id: UUID + name: str + code: str + description: str + roles: list[CourseRole] # Changed from list[str] + is_enrolled: bool + instructors: list[InstructorItem] +``` + +Update `CourseListView._get_view_data` (around line 120) to use `CourseRole` instead of strings. + +### 7. Modify detail.html Template + +**File**: `src/courses/templates/courses/detail.html` + +**Changes**: + +1. Add enrollment button section (after course description, before homework section): + +```html + +{% if student_profile and not data.is_enrolled and 'teacher' not in data.user_roles and 'teacher_assistant' not in data.user_roles %} +
+
+
+
+
You are not enrolled in this course
+

Enroll now to access homework assignments and participate in conversations.

+
+ {% csrf_token %} + +
+
+
+
+
+{% endif %} +``` + +2. Add instructors section (after description): + +```html + +{% if data.instructors %} +
+
+

Instructors

+ +
+
+{% endif %} +``` + +3. Update the "Enrolled" badge to use `is_enrolled`: + +```html +{% if data.is_enrolled %} +Enrolled +{% endif %} +``` + +## Edge Cases + +1. **Inactive course**: Students should NOT see an enrollment button for inactive courses. The enrollment form should check `course.is_active` before allowing enrollment. + +2. **Teacher viewing other teacher's course**: Teachers who don't teach a course should still see the course if they have the direct URL (they will see course info but no teacher-specific UI). + +3. **Unauthenticated users**: Should still be redirected to login (handled by `@login_required` decorator). + +4. **TA viewing course they assist**: TAs have full access, no enrollment button needed. + +5. **Already enrolled student**: Should see "Enrolled" badge, not enrollment button. + +## Testing Considerations + +### New Tests to Add + +1. **`test_student_can_view_unenrolled_course`**: Verify that a student who is NOT enrolled can access the course detail page (status 200). + +2. **`test_student_sees_enrollment_button_for_unenrolled_course`**: Verify that an unenrolled student sees the enrollment button. + +3. **`test_student_does_not_see_enrollment_button_for_inactive_course`**: Verify that enrollment button is not shown for inactive courses. + +4. **`test_student_sees_instructors_for_unenrolled_course`**: Verify that uninrolled students can see the instructors list. + +5. **`test_enrollment_redirects_to_course_detail`**: Verify that after enrolling, user is redirected to course detail page. + +### Existing Tests to Modify + +1. **`test_student_cannot_view_unenrolled_course`** (line 777-786): Change expected behavior from 403 to 200, and verify limited view with `is_enrolled=False`. + +2. **`test_course_detail_shows_correct_user_type`** (line 819-833): Update to use `CourseRole` enum values. + +## File Summary + +| File | Change Type | Description | +|------|-------------|-------------| +| `src/courses/enums.py` | Create | New file with `CourseRole` enum | +| `src/courses/views.py` | Modify | Update dataclasses, views to use `CourseRole` enum, add instructor data, modify enrollment redirect | +| `src/courses/templates/courses/detail.html` | Modify | Add enrollment button, instructors section, update badge logic | +| `src/courses/tests/test_views.py` | Modify | Update tests for new behavior and enum usage | + +## Implementation Order + +1. Create `src/courses/enums.py` with `CourseRole` enum +2. Update `CourseItem` and `CourseListData` to use `CourseRole` +3. Update `CourseDetailData` to include `instructors` and `is_enrolled` +4. Update `CourseDetailView.get()` to remove 403 for empty roles +5. Update `_get_view_data()` to return `instructors` and `is_enrolled` +6. Update `CourseEnrollView.post()` to redirect to course detail +7. Update `detail.html` template with enrollment button and instructors section +8. Update tests +9. Run tests to verify