Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/conversations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def section(self):
@property
def student(self):
"""Get the student through the conversation."""
return self.conversation.user.student_profile
return getattr(self.conversation.user, "student_profile", None)

def clean(self):
"""Ensure only one submission per student per section."""
Expand Down
7 changes: 4 additions & 3 deletions src/conversations/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ class SubmissionData:
conversation_id: UUID
section_id: UUID
section_title: str
student_id: UUID
student_id: UUID | None
student_name: str
submitted_at: datetime

Expand Down Expand Up @@ -904,8 +904,9 @@ def get_submission_data(
"conversation__user__student_profile", "conversation__section"
).get(id=submission_id)

# Get student's name
# Get student's name and profile
user = submission.conversation.user
student_profile = getattr(user, "student_profile", None)
student_name = f"{user.first_name} {user.last_name}"
if not student_name.strip():
student_name = user.username
Expand All @@ -916,7 +917,7 @@ def get_submission_data(
conversation_id=submission.conversation.id,
section_id=submission.conversation.section.id,
section_title=submission.conversation.section.title,
student_id=submission.conversation.user.student_profile.id,
student_id=student_profile.id if student_profile else None,
student_name=student_name,
submitted_at=submission.submitted_at,
)
Expand Down
8 changes: 8 additions & 0 deletions src/conversations/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,14 @@ def test_submission_student_property(self):
submission = Submission.objects.create(**self.submission_data)
self.assertEqual(submission.student, self.student)

def test_submission_student_property_with_teacher_conversation(self):
"""Submission.student should return None for teacher test conversations."""
teacher_conv = Conversation.objects.create(
user=self.teacher_user, section=self.section
)
submission = Submission.objects.create(conversation=teacher_conv)
self.assertIsNone(submission.student)

def test_submission_clean_method_validation(self):
"""Test submission clean method validation."""
# Create first submission
Expand Down
13 changes: 13 additions & 0 deletions src/conversations/tests/test_submission_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,19 @@ def test_get_submission_data(self):
self.assertEqual(data.student_id, self.student.id)
self.assertIn(self.student_user.username, data.student_name)

def test_get_submission_data_with_teacher_conversation(self):
"""get_submission_data should handle teacher-owned conversations."""
teacher_conv = Conversation.objects.create(
user=self.teacher_user, section=self.section
)
submission = Submission.objects.create(conversation=teacher_conv)

data = SubmissionService.get_submission_data(submission.id)

self.assertIsNotNone(data)
self.assertEqual(data.id, submission.id)
self.assertEqual(data.conversation_id, teacher_conv.id)

def test_get_student_submissions(self):
"""Test retrieving all submissions for a student."""
# Create two submissions for different sections
Expand Down
8 changes: 8 additions & 0 deletions src/homeworks/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ class SectionData:
status: SectionStatus | None = None
conversation_id: UUID | None = None
answer_count: int = 0
# Teacher test conversations (populated in view for teachers/TAs)
conversations: list[dict] | None = None

@property
def has_solution(self) -> bool:
Expand Down Expand Up @@ -678,6 +680,8 @@ def get_all_homework_matrix(teacher_id: UUID) -> HomeworkMatrixData | None:

# Populate conversation map
for conv in conversations:
if not hasattr(conv.user, "student_profile"):
continue
student_id = conv.user.student_profile.id
homework_id = conv.section.homework.id
key = (student_id, homework_id)
Expand Down Expand Up @@ -883,6 +887,8 @@ def get_course_homework_matrix(
submission_map = {sub.conversation.id: sub for sub in submissions}

for conv in conversations:
if not hasattr(conv.user, "student_profile"):
continue
student_id = conv.user.student_profile.id
homework_id = conv.section.homework.id
key = (student_id, homework_id)
Expand All @@ -897,6 +903,8 @@ def get_course_homework_matrix(
tuple[UUID, UUID], datetime
] = {}
for answer in section_answers:
if not hasattr(answer.user, "student_profile"):
continue
student_id = answer.user.student_profile.id
homework_id = answer.section.homework.id
key = (student_id, homework_id)
Expand Down
30 changes: 30 additions & 0 deletions src/homeworks/templates/homeworks/detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,36 @@ <h2 class="accordion-header" id="sec-heading-{{ forloop.counter }}">
</zero-md>
</div>

{% if 'teacher' in data.user_roles and section.section_type != 'non_interactive' %}
<div class="mb-3">
<a href="{% url 'conversations:start' section_id=section.id %}" class="btn btn-outline-primary btn-sm">
<i class="bi bi-play-circle"></i> Try Conversation
</a>
</div>
{% endif %}

{% if 'teacher_assistant' in data.user_roles and section.section_type != 'non_interactive' %}
<div class="mb-3">
<a href="{% url 'conversations:start' section_id=section.id %}" class="btn btn-outline-primary btn-sm">
<i class="bi bi-play-circle"></i> Test Conversation
</a>
</div>
{% endif %}

{% if section.conversations %}
<div class="mb-3">
<h6 class="text-muted mb-2">Previous Conversations</h6>
<div class="list-group">
{% for conv in section.conversations %}
<a href="{% url 'conversations:detail' conversation_id=conv.id %}" class="list-group-item list-group-item-action d-flex justify-content-between align-items-center py-2">
<span>{{ conv.label }}</span>
<span class="badge bg-secondary rounded-pill">{{ conv.message_count }} messages</span>
</a>
{% endfor %}
</div>
</div>
{% endif %}

{% if 'student' in data.user_roles %}
<div class="mb-3">
{% if data.widget_progress and not data.widget_progress.all_pre_answered %}
Expand Down
2 changes: 1 addition & 1 deletion src/homeworks/templates/homeworks/section_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ <h4 class="mb-0">Conversations</h4>
{% if data.conversations %}
<div class="list-group">
{% for conversation in data.conversations %}
<a href="#" class="list-group-item list-group-item-action d-flex justify-content-between align-items-center">
<a href="{% url 'conversations:detail' conversation_id=conversation.id %}" class="list-group-item list-group-item-action d-flex justify-content-between align-items-center">
<div>
<h5 class="mb-1">
{{ conversation.label }}
Expand Down
26 changes: 26 additions & 0 deletions src/homeworks/tests/test_matrix_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -650,3 +650,29 @@ def test_ta_not_shown_in_matrix(self):
"TA with student profile should not appear in matrix",
)
self.assertEqual(matrix_data.total_students, 2)

def test_matrix_with_teacher_test_conversations_ignored(self):
"""Teacher test conversations should not crash get_course_homework_matrix."""
Conversation.objects.create(user=self.teacher_user, section=self.section1_1)

matrix_data = HomeworkService.get_course_homework_matrix(self.course.id)

self.assertIsNotNone(matrix_data)
self.assertEqual(matrix_data.total_students, 2)
for student_row in matrix_data.student_rows:
for cell in student_row.homework_cells:
self.assertEqual(cell.submitted_sections, 0)
self.assertEqual(cell.total_conversations, 0)

def test_all_homework_matrix_with_teacher_conversations(self):
"""Teacher test conversations should not crash get_all_homework_matrix."""
Conversation.objects.create(user=self.teacher_user, section=self.section1_1)

matrix_data = HomeworkService.get_all_homework_matrix(self.teacher.id)

self.assertIsNotNone(matrix_data)
self.assertEqual(matrix_data.total_students, 2)
for student_row in matrix_data.student_rows:
for cell in student_row.homework_cells:
self.assertEqual(cell.submitted_sections, 0)
self.assertEqual(cell.total_conversations, 0)
33 changes: 33 additions & 0 deletions src/homeworks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -994,10 +994,42 @@ def _get_view_data(self, user, homework_id: UUID) -> HomeworkDetailData | None:
pass

if homework_detail.sections:
# Gather teacher/TA test conversations per section
teacher_conversations_map: dict[UUID, list[dict]] = {}
if "teacher" in user_roles or "teacher_assistant" in user_roles:
from conversations.models import Conversation as ConvModel

section_ids = [s.id for s in homework_detail.sections]
test_convs = (
ConvModel.objects.filter(
user=user,
section_id__in=section_ids,
is_deleted=False,
)
.select_related("section")
.prefetch_related("messages")
)
for conv in test_convs:
section_id = conv.section.id
if section_id not in teacher_conversations_map:
teacher_conversations_map[section_id] = []
teacher_conversations_map[section_id].append(
{
"id": conv.id,
"created_at": conv.created_at,
"updated_at": conv.updated_at,
"message_count": conv.message_count,
"label": f"Test conversation {conv.created_at.strftime('%Y-%m-%d %H:%M')}",
}
)

for section_data in homework_detail.sections:
# Get progress data for this section if available
progress: SectionData | None = section_progress_map.get(section_data.id)

# Get teacher test conversations for this section
convs = teacher_conversations_map.get(section_data.id)

sections.append(
SectionData(
id=section_data.id,
Expand All @@ -1011,6 +1043,7 @@ def _get_view_data(self, user, homework_id: UUID) -> HomeworkDetailData | None:
status=progress.status if progress else None,
conversation_id=progress.conversation_id if progress else None,
answer_count=progress.answer_count if progress else 0,
conversations=convs,
)
)

Expand Down
Loading