From 6e0e185ad8c0482a1bf74bca0c0d3db69517146b Mon Sep 17 00:00:00 2001 From: JS Ng Date: Wed, 30 Sep 2026 13:05:33 +0800 Subject: [PATCH] Fix nested resources dropping their path part Nested Resource classes (Submission.responses/feedback, Assignment.links, Timetable Entries/Metadata, Circle.members) were constructed without their path part, so their paths collapsed onto the parent resource's path and collection-style calls hit the parent URL with the wrong method (405). Also drop end_slash=True from Timetable Entries.list()/Metadata.get() so they request the API's leaf routes without trailing slashes, and pass Assignments.list() filters via the client's query argument. Fixes #38 --- campus_python/api/v1/assignments.py | 4 +- campus_python/api/v1/circles.py | 2 +- campus_python/api/v1/submissions.py | 4 +- campus_python/api/v1/timetable.py | 8 +- tests/unit/test_nested_resources.py | 158 ++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 tests/unit/test_nested_resources.py diff --git a/campus_python/api/v1/assignments.py b/campus_python/api/v1/assignments.py index bdc4556..a174135 100644 --- a/campus_python/api/v1/assignments.py +++ b/campus_python/api/v1/assignments.py @@ -29,7 +29,7 @@ def list(self, *, created_by: "str | None" = None) -> "list[campus.model.Assignm if created_by: params["created_by"] = created_by - resp = self.client.get(self.make_path(), params=params) + resp = self.client.get(self.make_path(), query=params) # Raise error if status code is not 2XX or 3XX resp.raise_for_status() return [ @@ -63,7 +63,7 @@ class Assignment(Resource): @property def links(self) -> "Assignments.Assignment.Links": """Get the links resource for this assignment.""" - return Assignments.Assignment.Links(parent=self) + return Assignments.Assignment.Links("links", parent=self) def delete(self) -> None: resp = self.client.delete(self.make_path()) diff --git a/campus_python/api/v1/circles.py b/campus_python/api/v1/circles.py index 6f87911..923ef7a 100644 --- a/campus_python/api/v1/circles.py +++ b/campus_python/api/v1/circles.py @@ -46,7 +46,7 @@ class Circle(Resource): @property def members(self) -> "Circles.Circle.CircleMembers": """Get the members resource for this circle.""" - return Circles.Circle.CircleMembers(parent=self) + return Circles.Circle.CircleMembers("members", parent=self) def delete(self) -> None: resp = self.client.delete(self.make_path()) diff --git a/campus_python/api/v1/submissions.py b/campus_python/api/v1/submissions.py index 2b6d82d..6dc7440 100644 --- a/campus_python/api/v1/submissions.py +++ b/campus_python/api/v1/submissions.py @@ -121,12 +121,12 @@ class Submission(Resource): @property def responses(self) -> "Submissions.Submission.Responses": """Get the responses resource for this submission.""" - return Submissions.Submission.Responses(parent=self) + return Submissions.Submission.Responses("responses", parent=self) @property def feedback(self) -> "Submissions.Submission.Feedback": """Get the feedback resource for this submission.""" - return Submissions.Submission.Feedback(parent=self) + return Submissions.Submission.Feedback("feedback", parent=self) def delete(self) -> None: """Delete this submission.""" diff --git a/campus_python/api/v1/timetable.py b/campus_python/api/v1/timetable.py index bc0c527..db2b5c6 100644 --- a/campus_python/api/v1/timetable.py +++ b/campus_python/api/v1/timetable.py @@ -138,12 +138,12 @@ class Timetable(Resource): @property def entries(self) -> "Timetables.Timetable.Entries": """Get the entries resource for this timetable.""" - return Timetables.Timetable.Entries(parent=self) + return Timetables.Timetable.Entries("entries", parent=self) @property def metadata(self) -> "Timetables.Timetable.Metadata": """Get the metadata resource for this timetable.""" - return Timetables.Timetable.Metadata(parent=self) + return Timetables.Timetable.Metadata("metadata", parent=self) def get(self) -> campus.model.Timetable: """Get the metadata for this timetable.""" @@ -160,7 +160,7 @@ class Entries(Resource): def list(self) -> "list[campus.model.TimetableEntry]": """Return a list of all entries for this timetable.""" - resp = self.client.get(self.make_path(end_slash=True)) + resp = self.client.get(self.make_path()) resp.raise_for_status() return [ campus.model.TimetableEntry.from_resource(item) @@ -176,7 +176,7 @@ def get(self) -> campus.model.TimetableMetadata: This does not include timetable entries. """ - resp = self.client.get(self.make_path(end_slash=True)) + resp = self.client.get(self.make_path()) resp.raise_for_status() return campus.model.Timetable.from_resource(resp.json()) diff --git a/tests/unit/test_nested_resources.py b/tests/unit/test_nested_resources.py new file mode 100644 index 0000000..732b67c --- /dev/null +++ b/tests/unit/test_nested_resources.py @@ -0,0 +1,158 @@ +"""Regression tests for nested resource paths (issue #38). + +Nested resources must be constructed with their path part so that their +paths extend the parent resource's path instead of collapsing onto it. + +The expected request paths mirror the Campus API routes defined in the +campus-suite repository (branch: weekly): +- POST /api/v1/submissions//responses +- POST /api/v1/submissions//feedback +- POST /api/v1/submissions//submit +- POST /api/v1/assignments//links +- GET /api/v1/timetable//entries +- GET /api/v1/timetable//metadata +- GET /api/v1/circles//members +""" + +import unittest +from unittest.mock import Mock, patch + +import campus.model + +from campus_python.api.v1 import ApiRoot + + +def make_api() -> tuple[ApiRoot, Mock]: + """Create an ApiRoot backed by a mock JSON client.""" + client = Mock() + return ApiRoot(json_client=client), client + + +class TestNestedResourcePaths(unittest.TestCase): + """Nested resource paths must include their own path part.""" + + def setUp(self): + self.api, _ = make_api() + + def test_submission_responses_path(self): + responses = self.api.submissions["sub-1"].responses + self.assertEqual( + responses.make_path(), "/api/v1/submissions/sub-1/responses" + ) + + def test_submission_feedback_path(self): + feedback = self.api.submissions["sub-1"].feedback + self.assertEqual( + feedback.make_path(), "/api/v1/submissions/sub-1/feedback" + ) + + def test_assignment_links_path(self): + links = self.api.assignments["asg-1"].links + self.assertEqual(links.make_path(), "/api/v1/assignments/asg-1/links") + + def test_timetable_entries_path(self): + entries = self.api.timetable["tt-1"].entries + self.assertEqual(entries.make_path(), "/api/v1/timetable/tt-1/entries") + + def test_timetable_metadata_path(self): + metadata = self.api.timetable["tt-1"].metadata + self.assertEqual(metadata.make_path(), "/api/v1/timetable/tt-1/metadata") + + def test_circle_members_path(self): + members = self.api.circles["cir-1"].members + self.assertEqual(members.make_path(), "/api/v1/circles/cir-1/members") + + def test_nested_paths_extend_parent_path(self): + """A nested resource path must not collapse onto its parent's path.""" + submission = self.api.submissions["sub-1"] + for child in (submission.responses, submission.feedback): + self.assertTrue(child.make_path().startswith(submission.make_path())) + self.assertNotEqual(child.make_path(), submission.make_path()) + + assignment = self.api.assignments["asg-1"] + self.assertTrue(assignment.links.make_path().startswith(assignment.make_path())) + self.assertNotEqual(assignment.links.make_path(), assignment.make_path()) + + timetable = self.api.timetable["tt-1"] + for child in (timetable.entries, timetable.metadata): + self.assertTrue(child.make_path().startswith(timetable.make_path())) + self.assertNotEqual(child.make_path(), timetable.make_path()) + + circle = self.api.circles["cir-1"] + self.assertTrue(circle.members.make_path().startswith(circle.make_path())) + self.assertNotEqual(circle.members.make_path(), circle.make_path()) + + +class TestNestedResourceRequests(unittest.TestCase): + """Nested resource methods must request their own API endpoints.""" + + def setUp(self): + self.api, self.client = make_api() + + def test_responses_add_posts_to_responses_endpoint(self): + self.api.submissions["sub-1"].responses.add( + question_id="q-1", response_text="answer" + ) + self.client.post.assert_called_once_with( + "/api/v1/submissions/sub-1/responses", + json={"question_id": "q-1", "response_text": "answer"}, + ) + + def test_feedback_add_posts_to_feedback_endpoint(self): + self.api.submissions["sub-1"].feedback.add( + question_id="q-1", feedback_text="good" + ) + self.client.post.assert_called_once_with( + "/api/v1/submissions/sub-1/feedback", + json={"question_id": "q-1", "feedback_text": "good"}, + ) + + def test_submit_posts_to_submit_endpoint(self): + self.api.submissions["sub-1"].submit() + self.client.post.assert_called_once_with( + "/api/v1/submissions/sub-1/submit" + ) + + def test_links_add_posts_to_links_endpoint(self): + self.api.assignments["asg-1"].links.add( + course_id="course-1", coursework_id="cw-1" + ) + self.client.post.assert_called_once_with( + "/api/v1/assignments/asg-1/links", + json={"course_id": "course-1", "coursework_id": "cw-1"}, + ) + + def test_entries_list_gets_entries_endpoint(self): + self.client.get.return_value.json.return_value = {"entries": []} + self.api.timetable["tt-1"].entries.list() + self.client.get.assert_called_once_with( + "/api/v1/timetable/tt-1/entries" + ) + + def test_metadata_get_gets_metadata_endpoint(self): + with patch.object( + campus.model.Timetable, "from_resource", return_value=Mock() + ): + self.api.timetable["tt-1"].metadata.get() + self.client.get.assert_called_once_with( + "/api/v1/timetable/tt-1/metadata" + ) + + def test_members_list_gets_members_endpoint(self): + self.client.get.return_value.json.return_value = {"members": {}} + self.api.circles["cir-1"].members.list() + self.client.get.assert_called_once_with( + "/api/v1/circles/cir-1/members" + ) + + def test_assignments_list_passes_query_argument(self): + """Assignments.list must pass filters via the client's query argument.""" + self.client.get.return_value.json.return_value = {"data": []} + self.api.assignments.list(created_by="teacher-1") + self.client.get.assert_called_once_with( + "/api/v1/assignments/", query={"created_by": "teacher-1"} + ) + + +if __name__ == "__main__": + unittest.main()