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
31 changes: 25 additions & 6 deletions canvigator_course.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,8 @@ def __init__(self, canvas, canvas_course, config, verbose=False):
if verbose:
print(self.students)

# use course_code prefix, course number, and CRN to create course_path
tmp_course_code = str(self.canvas_course.course_code)
course_path = tmp_course_code.split('-')[0] + tmp_course_code.split('-')[1] + "_" + tmp_course_code[-5:]
course_path = "/" + course_path.lower()
course_path = course_path.replace(" ", "")
self.config.addCourseToPath(course_path)
# Derive the data/figures subdirectory from the Canvas course_code.
self.config.addCourseToPath(_courseCodeToPath(self.canvas_course.course_code))
logger.info(f"Initialized course: {self.canvas_course.name}")

def getAllQuizzesAndSubmissions(self):
Expand Down Expand Up @@ -872,6 +868,29 @@ def deleteOldConversations(canvas, dry_run=False, max_age_months=6):
spin_done(summary + ".")


def _courseCodeToPath(course_code):
"""Derive the data/figures subdirectory ('/<dir>') from a Canvas course_code.

The canonical MSU Denver code looks like ``CSI-3300-001-12345`` and maps to
``/csi3300_12345`` — subject+number, an underscore, then the last 5 chars as
a CRN. Codes with at least two hyphen-separated parts keep that exact mapping
(so existing course directories are unchanged).

Codes with fewer than two hyphen-separated parts (e.g. ``MATH101`` or
``CS 3120 Spring 2026``) previously raised ``IndexError`` here, which blocked
every task for such a course; they now fall back to a sanitized form of the
whole code so course selection never crashes on an unusual code. Spaces are
stripped from the result in all cases.
"""
code = str(course_code or '').strip()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep blank course codes out of root data path

When Canvas returns a blank or None course_code, this normalization turns it into '', and the helper returns "/"; CanvigatorConfig.addCourseToPath then strips that to an empty directory name and leaves data_path/figures_path at the shared data and figures roots. In that scenario, exports and later find_latest_csv lookups for the course are no longer isolated from other courses' files, so a missing Canvas code can mix or read the wrong course artifacts instead of getting a unique fallback directory.

Useful? React with 👍 / 👎.

parts = code.split('-')
if len(parts) >= 2:
stem = parts[0] + parts[1] + "_" + code[-5:]
else:
stem = code
return ("/" + stem.lower()).replace(" ", "")


def _resolveCourseStart(canvas_course):
"""Return the course start as a UTC datetime, or None if unavailable.

Expand Down
16 changes: 14 additions & 2 deletions canvigator_quiz.py
Original file line number Diff line number Diff line change
Expand Up @@ -1278,6 +1278,16 @@ def _saveReminderManifest(self, messages, subject_str, dry_run):
print(f" Manifest saved: {csv_name.name}")
logger.info(f"Reminder manifest saved: {csv_name}")

# Canonical column order for the followup_replies CSV. Passed explicitly to
# pd.DataFrame so the file always carries a header row — even when no student
# has replied yet — so the downstream re-read in _loadFollowUpReplies can't
# hit pandas' EmptyDataError on a column-less file.
REPLIES_COLUMNS = [
'student_id', 'student_name', 'question_id', 'question_mode',
'conversation_id', 'message_id', 'reply_text', 'has_attachment',
'attachment_path', 'has_audio', 'audio_path', 'replied_at', 'latest',
]

def getFollowUpReplies(self, reply_window_days=5):
"""Retrieve student replies to follow-up questions from Canvas conversations.

Expand Down Expand Up @@ -1364,8 +1374,10 @@ def getFollowUpReplies(self, reply_window_days=5):
'latest': is_latest,
})

# Save the replies CSV
replies_df = pd.DataFrame(all_replies)
# Save the replies CSV (explicit columns so an empty all_replies still
# writes a header row rather than a column-less file that would crash
# the re-read in _loadFollowUpReplies with EmptyDataError).
replies_df = pd.DataFrame(all_replies, columns=self.REPLIES_COLUMNS)
csv_name = self.config.data_path / f"{file_prefix}followup_replies_{today_str()}.csv"
replies_df.to_csv(csv_name, index=False)

Expand Down
75 changes: 75 additions & 0 deletions test_canvigator.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,41 @@ def test_due_at_exactly_now(self):
assert is_quiz_open_for_reminder(quiz, now=self._now()) is False


# ---------------------------------------------------------------------------
# canvigator_course path-derivation tests
# ---------------------------------------------------------------------------

class TestCourseCodeToPath:
"""Tests for canvigator_course._courseCodeToPath."""

def test_canonical_four_part_code(self):
"""The canonical 'SUBJ-NUM-SEC-CRN' code maps to '/subjnum_crn'."""
from canvigator_course import _courseCodeToPath
assert _courseCodeToPath('CSI-3300-001-12345') == '/csi3300_12345'

def test_no_hyphen_code_does_not_crash(self):
"""A code with no hyphens falls back to the sanitized whole code (regression)."""
from canvigator_course import _courseCodeToPath
assert _courseCodeToPath('MATH101') == '/math101'

def test_space_separated_code_does_not_crash(self):
"""A space-separated code with no hyphens is sanitized (spaces removed)."""
from canvigator_course import _courseCodeToPath
assert _courseCodeToPath('CS 3120 Spring 2026') == '/cs3120spring2026'

def test_two_part_code_preserves_legacy_mapping(self):
"""A two-part code keeps the existing subject+number + last-5 mapping."""
from canvigator_course import _courseCodeToPath
# last-5 of 'CS-3120' is '-3120'; preserved for backward compatibility.
assert _courseCodeToPath('CS-3120') == '/cs3120_-3120'

def test_none_and_empty_are_safe(self):
"""A None or empty course_code yields '/' without raising."""
from canvigator_course import _courseCodeToPath
assert _courseCodeToPath(None) == '/'
assert _courseCodeToPath('') == '/'


# ---------------------------------------------------------------------------
# canvigator_course anonymization tests
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -1212,6 +1247,46 @@ def test_preserves_newest_first_order(self):
assert result[1]['id'] == 1


class TestFollowUpRepliesEmptyRoundTrip:
"""Regression: a no-reply followup_replies CSV must round-trip without crashing.

When no student has replied yet, getFollowUpReplies writes a header-only CSV
(built from REPLIES_COLUMNS); _loadFollowUpReplies must read it back as an
empty frame so assessFollowUpReplies can print 'No student replies to assess.'
instead of raising pandas' EmptyDataError on a column-less file.
"""

def _make_quiz(self, tmp_path):
"""Build a CanvigatorQuiz bypassing __init__, pointed at tmp_path for I/O."""
from canvigator_quiz import CanvigatorQuiz

class _Config:
def __init__(self, data_path):
self.data_path = data_path
self.quiz_prefix = 'quiz'

class _CanvasQuiz:
id = 999

quiz = CanvigatorQuiz.__new__(CanvigatorQuiz)
quiz.canvas_quiz = _CanvasQuiz()
quiz.config = _Config(tmp_path)
return quiz

def test_empty_replies_csv_loads_without_error(self, tmp_path):
"""A header-only replies CSV (no data rows) loads as an empty frame with a 'latest' column."""
from canvigator_quiz import CanvigatorQuiz
quiz = self._make_quiz(tmp_path)
# Mirror the fixed writer: empty list + explicit columns -> header-only CSV.
csv_path = tmp_path / "quiz999_followup_replies_20260602.csv"
pd.DataFrame([], columns=CanvigatorQuiz.REPLIES_COLUMNS).to_csv(csv_path, index=False)

df = quiz._loadFollowUpReplies() # must NOT raise EmptyDataError
assert 'latest' in df.columns
latest_replies = df[df['latest'] == True].to_dict('records') # noqa: E712
assert latest_replies == []


class TestAssessmentsMerge:
"""Tests for CanvigatorQuiz._mergeAssessments and _indexAssessments."""

Expand Down
Loading