diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 6474efc1d374..1686b6539bf9 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -52,6 +52,7 @@ generate_anonymous_ids_for_course ) from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order +from django.db.models import Q log = logging.getLogger(__name__) @@ -82,12 +83,33 @@ def get_instructor_task_history(course_id, usage_key=None, student=None, task_ty that optionally match a particular problem, a student, and/or a task type. """ instructor_tasks = InstructorTask.objects.filter(course_id=course_id) + if usage_key is not None or student is not None: _, task_key = encode_problem_and_student_input(usage_key, student) instructor_tasks = instructor_tasks.filter(task_key=task_key) if task_type is not None: instructor_tasks = instructor_tasks.filter(task_type=task_type) + # Bulk email history is user-facing; only show tasks that represent + # real delivered emails (SUCCESS with succeeded > 0) or future scheduled sends. + if task_type == InstructorTaskTypes.BULK_COURSE_EMAIL: + instructor_tasks = InstructorTask.objects.filter( + course_id=course_id + ).filter( + # SUCCESS tasks must have delivery results, while SCHEDULED tasks + # have no task_output yet and must be included explicitly. + Q( + task_state='SUCCESS', + task_output__contains='"succeeded":' + ) | + Q( + task_state='SCHEDULED' + ) + ).exclude( + # Exclude completed tasks where no emails were actually sent + task_output__contains='"succeeded": 0' + ) + return instructor_tasks.order_by('-id') diff --git a/lms/djangoapps/instructor_task/tests/test_get_instructor_task_history.py b/lms/djangoapps/instructor_task/tests/test_get_instructor_task_history.py new file mode 100644 index 000000000000..15be7adf9b51 --- /dev/null +++ b/lms/djangoapps/instructor_task/tests/test_get_instructor_task_history.py @@ -0,0 +1,203 @@ +""" +Tests for get_instructor_task_history in bulk email. +""" +import json +from celery.states import SUCCESS, FAILURE, REVOKED + +from lms.djangoapps.instructor_task.api import get_instructor_task_history +from lms.djangoapps.instructor_task.tests.test_base import InstructorTaskCourseTestCase +from lms.djangoapps.instructor_task.tests.factories import InstructorTaskFactory + + +class TestGetInstructorTaskHistory(InstructorTaskCourseTestCase): + """ + Tests for updated filtering logic in get_instructor_task_history + + Rules: + - SUCCESS tasks must contain succeeded > 0 in task_output + - SCHEDULED tasks must be included even if task_output is empty + - SUCCESS tasks with succeeded = 0 must be excluded + - FAILED / REVOKED tasks must be excluded + """ + + def setUp(self): + super().setUp() + self.initialize_course() + self.instructor = self.create_instructor('instructor') + + def test_includes_successful_bulk_email_task(self): + """ + SUCCESS + succeeded > 0 → INCLUDED + """ + task_output = json.dumps({ + "attempted": 10, + "succeeded": 10, + "failed": 0 + }) + + success_task = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_success", + task_input='{}', + task_state=SUCCESS, + task_output=task_output, + task_key='bulk_email_success', + requester=self.instructor + ) + + tasks = list(get_instructor_task_history( + self.course.id, + task_type="bulk_course_email" + )) + + assert success_task in tasks + + def test_includes_scheduled_task_with_empty_output(self): + """ + SCHEDULED (even with empty {}) → INCLUDED + """ + scheduled_task = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_scheduled", + task_input='{}', + task_state="SCHEDULED", + task_output="{}", + task_key='bulk_email_scheduled', + requester=self.instructor + ) + + tasks = list(get_instructor_task_history( + self.course.id, + task_type="bulk_course_email" + )) + + assert scheduled_task in tasks + + def test_excludes_zero_success_tasks(self): + """ + SUCCESS + succeeded = 0 → EXCLUDED + """ + zero_success_task = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_zero", + task_state=SUCCESS, + task_output=json.dumps({ + "attempted": 10, + "succeeded": 0, + "failed": 10 + }), + task_key='bulk_email_zero', + requester=self.instructor + ) + + tasks = list(get_instructor_task_history( + self.course.id, + task_type="bulk_course_email" + )) + + assert zero_success_task not in tasks + + def test_excludes_failed_tasks(self): + """ + FAILURE → EXCLUDED + """ + failed_task = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_failed", + task_state=FAILURE, + task_output=json.dumps({ + "attempted": 5, + "succeeded": 0, + "failed": 5 + }), + task_key='bulk_email_failed', + requester=self.instructor + ) + + tasks = list(get_instructor_task_history( + self.course.id, + task_type="bulk_course_email" + )) + + assert failed_task not in tasks + + def test_excludes_revoked_tasks(self): + """ + REVOKED → EXCLUDED + """ + revoked_task = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_revoked", + task_state=REVOKED, + task_output='{"message": "Task revoked"}', + task_key='bulk_email_revoked', + requester=self.instructor + ) + + tasks = list(get_instructor_task_history( + self.course.id, + task_type="bulk_course_email" + )) + + assert revoked_task not in tasks + + def test_only_valid_tasks_returned(self): + """ + Only the following should be returned: + - SUCCESS with succeeded > 0 + - SCHEDULED + + Everything else must be excluded. + """ + valid_success = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_valid", + task_state=SUCCESS, + task_output=json.dumps({ + "attempted": 8, + "succeeded": 5, + "failed": 3 + }), + task_key='bulk_email_valid', + requester=self.instructor + ) + + scheduled = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_scheduled_2", + task_state="SCHEDULED", + task_output="{}", + task_key='bulk_email_scheduled_2', + requester=self.instructor + ) + + zero_task = InstructorTaskFactory.create( + course_id=self.course.id, + task_type="bulk_course_email", + task_id="bulk_email_zero_2", + task_state=SUCCESS, + task_output=json.dumps({ + "attempted": 5, + "succeeded": 0, + "failed": 5 + }), + task_key='bulk_email_zero_2', + requester=self.instructor + ) + + tasks = list(get_instructor_task_history( + self.course.id, + task_type="bulk_course_email" + )) + + assert valid_success in tasks + assert scheduled in tasks + assert zero_task not in tasks + assert len(tasks) == 2