diff --git a/canvigator_course.py b/canvigator_course.py index ad0ac72..a7a95b0 100644 --- a/canvigator_course.py +++ b/canvigator_course.py @@ -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): @@ -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 ('/') 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() + 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. diff --git a/canvigator_quiz.py b/canvigator_quiz.py index 8fbc3dc..3e81f0a 100644 --- a/canvigator_quiz.py +++ b/canvigator_quiz.py @@ -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. @@ -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) diff --git a/test_canvigator.py b/test_canvigator.py index 261f0f7..0786d83 100644 --- a/test_canvigator.py +++ b/test_canvigator.py @@ -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 # --------------------------------------------------------------------------- @@ -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."""