diff --git a/lms/tasks/roster.py b/lms/tasks/roster.py index e012b20191..808d87c76c 100644 --- a/lms/tasks/roster.py +++ b/lms/tasks/roster.py @@ -11,6 +11,8 @@ CourseRoster, Event, LMSCourse, + LMSSegment, + LMSSegmentRoster, TaskDone, ) from lms.services.roster import RosterService @@ -30,6 +32,7 @@ def schedule_fetching_rosters() -> None: schedule_fetching_course_rosters() schedule_fetching_assignment_rosters() + schedule_fetching_segment_rosters() def schedule_fetching_course_rosters() -> None: @@ -156,6 +159,69 @@ def schedule_fetching_assignment_rosters() -> None: ) +def schedule_fetching_segment_rosters() -> None: + """Schedule fetching segment rosters based on their last lunches and the most recent roster fetch.""" + + # We use the python version (and not func.now()) for easier mocking during tests + now = datetime.now() + + # Only fetch roster for segments for which we haven't schedule a fetch recently + no_recent_scheduled_roster_fetch_clause = ~exists( + select(TaskDone).where( + TaskDone.key == func.concat("roster::segment::scheduled::", LMSSegment.id), + ) + ) + + # Only fetch roster for segments that don't have recent roster information + no_recent_roster_clause = ~exists( + select(LMSSegmentRoster).where( + LMSSegmentRoster.lms_segment_id == LMSSegment.id, + LMSSegmentRoster.updated >= now - ROSTER_REFRESH_WINDOW, + ) + ) + + # Only fetch rosters for segments that belong to courses have been recently launched + recent_launches_clause = exists( + select(Event) + .join(Course, Event.course_id == Course.id) + .where( + Event.timestamp >= now - LAUNCHED_WINDOW, + Course.authority_provided_id == LMSCourse.h_authority_provided_id, + ) + ) + + with app.request_context() as request: + with request.tm: + query = ( + select(LMSSegment.id) + .join(LMSCourse, LMSSegment.lms_course_id == LMSCourse.id) + .where( + # Courses for which we have a LTIA membership service URL + LMSCourse.lti_context_memberships_url.is_not(None), + no_recent_roster_clause, + no_recent_scheduled_roster_fetch_clause, + recent_launches_clause, + # Only canvas groups for now + LMSSegment.type == "canvas_group", + ) + # Prefer newer segments + .order_by(LMSSegment.created.desc()) + # Schedule only a few rosters per call to this method + .limit(ROSTER_LIMIT) + ) + for lms_segment_id in request.db.scalars(query).all(): + fetch_segment_roster.delay(lms_segment_id=lms_segment_id) + # Record that the roster fetching has been scheduled + # We set the expiration date to ROSTER_REFRESH_WINDOW so we'll try again after that period + request.db.add( + TaskDone( + key=f"roster::segment::scheduled::{lms_segment_id}", + data=None, + expires_at=datetime.now() + ROSTER_REFRESH_WINDOW, + ) + ) + + @app.task( acks_late=True, autoretry_for=(Exception,), @@ -180,9 +246,25 @@ def fetch_course_roster(*, lms_course_id) -> None: retry_backoff_max=7200, ) def fetch_assignment_roster(*, assignment_id) -> None: - """Fetch the roster for one course.""" + """Fetch the roster for one assignment.""" with app.request_context() as request: roster_service: RosterService = request.find_service(RosterService) with request.tm: assignment = request.db.get(Assignment, assignment_id) roster_service.fetch_assignment_roster(assignment) + + +@app.task( + acks_late=True, + autoretry_for=(Exception,), + max_retries=2, + retry_backoff=3600, + retry_backoff_max=7200, +) +def fetch_segment_roster(*, lms_segment_id) -> None: + """Fetch the roster for one segment.""" + with app.request_context() as request: + roster_service: RosterService = request.find_service(RosterService) + with request.tm: + assignment = request.db.get(LMSSegment, lms_segment_id) + roster_service.fetch_canvas_group_roster(assignment) diff --git a/tests/unit/lms/tasks/roster_test.py b/tests/unit/lms/tasks/roster_test.py index 7774f39f83..5ed7776e6f 100644 --- a/tests/unit/lms/tasks/roster_test.py +++ b/tests/unit/lms/tasks/roster_test.py @@ -7,9 +7,11 @@ from lms.tasks.roster import ( fetch_assignment_roster, fetch_course_roster, + fetch_segment_roster, schedule_fetching_assignment_rosters, schedule_fetching_course_rosters, schedule_fetching_rosters, + schedule_fetching_segment_rosters, ) from tests import factories @@ -31,13 +33,25 @@ def test_fetch_assignment_roster(self, roster_service, db_session): roster_service.fetch_assignment_roster.assert_called_once_with(assignment) + def test_fetch_segment_roster(self, roster_service, db_session): + lms_segment = factories.LMSSegment() + db_session.flush() + + fetch_segment_roster(lms_segment_id=lms_segment.id) + + roster_service.fetch_canvas_group_roster.assert_called_once_with(lms_segment) + def test_schedule_fetching_rosters( - self, schedule_fetching_assignment_rosters, schedule_fetching_course_rosters + self, + schedule_fetching_assignment_rosters, + schedule_fetching_course_rosters, + schedule_fetching_segment_rosters, ): schedule_fetching_rosters() schedule_fetching_course_rosters.assert_called_once_with() schedule_fetching_assignment_rosters.assert_called_once_with() + schedule_fetching_segment_rosters.assert_called_once_with() @freeze_time("2024-08-28") @pytest.mark.usefixtures( @@ -77,6 +91,24 @@ def test_schedule_fetching_assignment_rosters( assignment_id=assignment_with_recent_launch.id ) + @freeze_time("2024-08-28") + @pytest.mark.usefixtures( + "lms_segment_with_no_launch", + "lms_segment_with_no_recent_launch", + "lms_segment_with_recent_launch_and_task_done_row", + "lms_segment_with_launch_and_recent_roster", + ) + def test_schedule_fetching_segment_rosters( + self, lms_segment_with_recent_launch, db_session, fetch_segment_roster + ): + db_session.flush() + + schedule_fetching_segment_rosters() + + fetch_segment_roster.delay.assert_called_once_with( + lms_segment_id=lms_segment_with_recent_launch.id + ) + @pytest.fixture def lms_course_with_no_service_url(self): return factories.LMSCourse() @@ -213,6 +245,51 @@ def assignment_with_launch_and_recent_roster(self, lms_course_with_recent_launch return assignment + @pytest.fixture + def lms_segment_with_no_launch(self, lms_course_with_no_launch): + return factories.LMSSegment( + lms_course=lms_course_with_no_launch, type="canvas_group" + ) + + @pytest.fixture + def lms_segment_with_no_recent_launch(self, lms_course_with_no_recent_launch): + return factories.LMSSegment( + lms_course=lms_course_with_no_recent_launch, type="canvas_group" + ) + + @pytest.fixture + def lms_segment_with_recent_launch(self, lms_course_with_recent_launch): + return factories.LMSSegment( + lms_course=lms_course_with_recent_launch, type="canvas_group" + ) + + @pytest.fixture + def lms_segment_with_recent_launch_and_task_done_row( + self, lms_course_with_recent_launch, db_session + ): + lms_segment = factories.LMSSegment( + lms_course=lms_course_with_recent_launch, type="canvas_group" + ) + db_session.flush() # Make sure we have an ID for the course + factories.TaskDone(key=f"roster::segment::scheduled::{lms_segment.id}") + + return lms_segment + + @pytest.fixture + def lms_segment_with_launch_and_recent_roster(self, lms_course_with_recent_launch): + lms_segment = factories.LMSSegment( + lms_course=lms_course_with_recent_launch, type="canvas_group" + ) + factories.LMSSegmentRoster( + lms_segment=lms_segment, + lms_user=factories.LMSUser(), + lti_role=factories.LTIRole(), + active=True, + updated=datetime(2024, 8, 25), + ) + + return lms_segment + @pytest.fixture def fetch_course_roster(self, patch): return patch("lms.tasks.roster.fetch_course_roster") @@ -221,6 +298,14 @@ def fetch_course_roster(self, patch): def fetch_assignment_roster(self, patch): return patch("lms.tasks.roster.fetch_assignment_roster") + @pytest.fixture + def fetch_segment_roster(self, patch): + return patch("lms.tasks.roster.fetch_segment_roster") + + @pytest.fixture + def schedule_fetching_segment_rosters(self, patch): + return patch("lms.tasks.roster.schedule_fetching_segment_rosters") + @pytest.fixture def schedule_fetching_assignment_rosters(self, patch): return patch("lms.tasks.roster.schedule_fetching_assignment_rosters")