Skip to content

Commit

Permalink
Fix query to find assignment rosters that need fetching
Browse files Browse the repository at this point in the history
The existing query used a subquery that joined itself on assignments.
That meant that that condition will return true if any event can be
joined with an assignments, that's always true.

Avoid the join and use the parent query Assignment, making the subquery
a correlated one to get the correct result.
  • Loading branch information
marcospri committed Nov 26, 2024
1 parent 1f28c64 commit 6d1005a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
5 changes: 2 additions & 3 deletions lms/tasks/roster.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ def schedule_fetching_assignment_rosters() -> None:

# Only fetch rosters for assignments that have been recently launched
recent_launches_clause = exists(
select(Event)
.join(Assignment, Event.assignment_id == Assignment.id)
.where(
select(Event).where(
Event.assignment_id == Assignment.id,
Event.timestamp >= now - LAUNCHED_WINDOW,
)
)
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/lms/tasks/roster_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def test_schedule_fetching_rosters(
@freeze_time("2024-08-28")
@pytest.mark.usefixtures(
"lms_course_with_no_launch",
"lms_course_with_no_recent_launch",
"lms_course_with_no_service_url",
"lms_course_with_launch_and_recent_roster",
"lms_course_with_recent_launch_and_task_done_row",
Expand All @@ -60,6 +61,7 @@ def test_schedule_fetching_course_rosters(
@freeze_time("2024-08-28")
@pytest.mark.usefixtures(
"assignment_with_no_launch",
"assignment_with_no_recent_launch",
"assignment_with_no_lti_v13_id",
"assignment_with_recent_launch_and_task_done_row",
"assignment_with_launch_and_recent_roster",
Expand Down Expand Up @@ -93,6 +95,17 @@ def assignment_with_no_launch(self):
lti_v13_resource_link_id="ID", course=factories.Course()
)

@pytest.fixture
def assignment_with_no_recent_launch(self, lms_course_with_recent_launch):
assignment = factories.Assignment(
lti_v13_resource_link_id="ID", course=lms_course_with_recent_launch.course
)
factories.Event(
assignment=assignment,
timestamp=datetime(2024, 1, 1),
)
return assignment

@pytest.fixture
def lms_course_with_recent_launch(self):
course = factories.Course()
Expand All @@ -107,6 +120,20 @@ def lms_course_with_recent_launch(self):
course=course,
)

@pytest.fixture
def lms_course_with_no_recent_launch(self):
course = factories.Course()
factories.Event(
course=course,
timestamp=datetime(2024, 1, 1),
)

return factories.LMSCourse(
lti_context_memberships_url="URL",
h_authority_provided_id=course.authority_provided_id,
course=course,
)

@pytest.fixture
def lms_course_with_recent_launch_and_task_done_row(self, db_session):
course = factories.Course()
Expand Down

0 comments on commit 6d1005a

Please sign in to comment.