From 1a0a3820f26e4df25e0fe12ab846e2eb2ff3d4fb Mon Sep 17 00:00:00 2001 From: David Doty Date: Wed, 16 Sep 2026 18:21:56 -0700 Subject: [PATCH] fix: stop get_course_users swallowing roster parse errors get_course_users wrapped its scrape in `except Exception: return None`, so any parsing failure surfaced only as `TypeError: 'NoneType' object is not iterable` at the caller's loop, with no indication of what actually went wrong. Its annotated return type is `list[Member]`, so let the exception propagate instead. Three roster rows could reach that path, all of them non-student members: - `user_id` is read from the js-rosterName button's `data-url`. The button is guarded for None but `data-url` itself is not, so a row carrying the button without that attribute raised AttributeError on `data_url.split(...)`. - the submissions count cell is blank for members who cannot submit (instructors, TAs, readers), so `int(cell.text)` raised ValueError. - `id_to_role[...]` raised KeyError for any role id outside {0,1,2,3}. Each now degrades for that one member -- `user_id=None`, `num_submissions=0`, or the raw role id passed through -- rather than taking down the whole roster. Adds offline parser tests covering student rows, staff rows, a name button with no data-url, an unrecognised role id, and a mixed roster; these need no Gradescope credentials. Co-Authored-By: Claude Opus 5 (1M context) --- .../classes/_helpers/_course_helpers.py | 19 ++-- src/gradescopeapi/classes/account.py | 15 +-- tests/test_course_members.py | 97 +++++++++++++++++++ 3 files changed, 115 insertions(+), 16 deletions(-) create mode 100644 tests/test_course_members.py diff --git a/src/gradescopeapi/classes/_helpers/_course_helpers.py b/src/gradescopeapi/classes/_helpers/_course_helpers.py index 23ab416..f2e6a7b 100644 --- a/src/gradescopeapi/classes/_helpers/_course_helpers.py +++ b/src/gradescopeapi/classes/_helpers/_course_helpers.py @@ -155,7 +155,9 @@ def get_course_members(soup: BeautifulSoup, course_id: str) -> list[Member]: # fetch other attributes: email, role, and section # from data attributes in button email = data_button.get("data-email") - role = id_to_role[data_button.get("data-role")] + # an unrecognised role id passes through as-is rather than sinking the whole roster + role_id = data_button.get("data-role") + role = id_to_role.get(role_id, role_id) sections = data_button.get("data-sections") # TODO: check if this is correct # fetch user_id, only available on 'student' accounts @@ -164,11 +166,16 @@ def get_course_members(soup: BeautifulSoup, course_id: str) -> list[Member]: user_id = None if rosterName_button is not None: # data-url="/courses/753413/gradebook.json?user_id=6515875" - data_url: str = rosterName_button.get("data-url", None) - user_id = data_url.split("user_id=")[-1] - - # fetch number of submissions from table cell - num_submissions = int(cells[num_submissions_column].text) + data_url: str | None = rosterName_button.get("data-url", None) + if data_url is not None and "user_id=" in data_url: + user_id = data_url.split("user_id=")[-1] + + # fetch number of submissions from table cell; + # the cell is blank for members who cannot submit (e.g. instructors, TAs, readers) + num_submissions_text = cells[num_submissions_column].text.strip() + num_submissions = ( + int(num_submissions_text) if num_submissions_text.isdigit() else 0 + ) # create Member object with all relevant info member_list.append( diff --git a/src/gradescopeapi/classes/account.py b/src/gradescopeapi/classes/account.py index 93467a4..c260647 100644 --- a/src/gradescopeapi/classes/account.py +++ b/src/gradescopeapi/classes/account.py @@ -88,17 +88,12 @@ def get_course_users(self, course_id: str) -> list[Member]: session = self.session - try: - # scrape page - membership_resp = check_page_auth(session, membership_endpoint) - membership_soup = BeautifulSoup(membership_resp.text, "html.parser") - - # get all users in the course - users = get_course_members(membership_soup, course_id) + # scrape page + membership_resp = check_page_auth(session, membership_endpoint) + membership_soup = BeautifulSoup(membership_resp.text, "html.parser") - return users - except Exception: - return None + # get all users in the course + return get_course_members(membership_soup, course_id) def get_assignments(self, course_id: str) -> list[Assignment]: """ diff --git a/tests/test_course_members.py b/tests/test_course_members.py new file mode 100644 index 0000000..d9b704d --- /dev/null +++ b/tests/test_course_members.py @@ -0,0 +1,97 @@ +"""Tests for roster parsing in get_course_members. + +These are offline tests against fixture HTML, so they need no Gradescope credentials. +""" + +from bs4 import BeautifulSoup + +from gradescopeapi.classes._helpers._course_helpers import get_course_members + +COURSE_ID = "753413" + + +def _roster(rows: str) -> BeautifulSoup: + html = f""" + + + + {rows} +
NameEmailRoleSectionsSubmissionsEditRemove
""" + return BeautifulSoup(html, "html.parser") + + +def _row(data_cm: str, role: str, name_button: str, submissions: str) -> str: + return f""" + + + + {name_button} + + ers{submissions}editremove + """ + + +STUDENT_CM = '{"full_name":"Test Student","first_name":"Test","last_name":"Student","sid":"912345678"}' +STAFF_CM = '{"full_name":"Test Instructor","first_name":"Test","last_name":"Instructor","sid":null}' + + +def test_student_row_parses_user_id_and_submissions(): + """A student row has a gradebook data-url, which is where user_id comes from.""" + name_button = ( + '' + ) + (member,) = get_course_members( + _roster(_row(STUDENT_CM, "0", name_button, "7")), COURSE_ID + ) + + assert member.full_name == "Test Student" + assert member.role == "Student" + assert member.sid == "912345678" + assert member.user_id == "6515875" + assert member.num_submissions == 7 + + +def test_staff_row_without_name_button_has_no_user_id(): + """Only student accounts expose a user_id, and staff submission cells are blank.""" + (member,) = get_course_members(_roster(_row(STAFF_CM, "1", "", "")), COURSE_ID) + + assert member.full_name == "Test Instructor" + assert member.role == "Instructor" + assert member.user_id is None + assert member.num_submissions == 0 + + +def test_name_button_without_data_url_has_no_user_id(): + """A js-rosterName button need not carry a data-url; that must not raise.""" + name_button = '' + (member,) = get_course_members( + _roster(_row(STAFF_CM, "2", name_button, "—")), COURSE_ID + ) + + assert member.role == "TA" + assert member.user_id is None + assert member.num_submissions == 0 + + +def test_unrecognised_role_id_passes_through(): + """A role id outside the known set must not sink the roster.""" + (member,) = get_course_members(_roster(_row(STAFF_CM, "9", "", "")), COURSE_ID) + + assert member.role == "9" + assert member.full_name == "Test Instructor" + + +def test_mixed_roster_returns_every_member(): + """One unparseable row used to sink the whole roster.""" + student_button = ( + '' + ) + rows = _row(STUDENT_CM, "0", student_button, "7") + _row(STAFF_CM, "1", "", "") + members = get_course_members(_roster(rows), COURSE_ID) + + assert [m.role for m in members] == ["Student", "Instructor"] + assert [m.user_id for m in members] == ["6515875", None]