Skip to content

Commit 5b5b586

Browse files
ajayakkalasettiajayakkalasetti
committed
fix: filter out unsent or revoked tasks from Bulk Email Email Task History
1 parent d97e943 commit 5b5b586

2 files changed

Lines changed: 220 additions & 0 deletions

File tree

‎lms/djangoapps/instructor_task/api.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
generate_anonymous_ids_for_course
5353
)
5454
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
55+
from django.db.models import Q
5556

5657
log = logging.getLogger(__name__)
5758

@@ -82,12 +83,28 @@ def get_instructor_task_history(course_id, usage_key=None, student=None, task_ty
8283
that optionally match a particular problem, a student, and/or a task type.
8384
"""
8485
instructor_tasks = InstructorTask.objects.filter(course_id=course_id)
86+
8587
if usage_key is not None or student is not None:
8688
_, task_key = encode_problem_and_student_input(usage_key, student)
8789
instructor_tasks = instructor_tasks.filter(task_key=task_key)
8890
if task_type is not None:
8991
instructor_tasks = instructor_tasks.filter(task_type=task_type)
9092

93+
if task_type == InstructorTaskTypes.BULK_COURSE_EMAIL:
94+
instructor_tasks = InstructorTask.objects.filter(
95+
course_id=course_id
96+
).filter(
97+
Q(
98+
task_state='SUCCESS',
99+
task_output__contains='"succeeded":'
100+
) |
101+
Q(
102+
task_state='SCHEDULED'
103+
)
104+
).exclude(
105+
task_output__contains='"succeeded": 0'
106+
)
107+
91108
return instructor_tasks.order_by('-id')
92109

93110

‎lms/djangoapps/instructor_task/tests/test_get_instructor_task_history.py‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
"""
2+
Tests for get_instructor_task_history in bulk email.
3+
"""
4+
import json
5+
from celery.states import SUCCESS, FAILURE, REVOKED
6+
7+
from lms.djangoapps.instructor_task.api import get_instructor_task_history
8+
from lms.djangoapps.instructor_task.tests.test_base import InstructorTaskCourseTestCase
9+
from lms.djangoapps.instructor_task.tests.factories import InstructorTaskFactory
10+
11+
12+
class TestGetInstructorTaskHistory(InstructorTaskCourseTestCase):
13+
"""
14+
Tests for updated filtering logic in get_instructor_task_history
15+
16+
Rules:
17+
- SUCCESS tasks must contain succeeded > 0 in task_output
18+
- SCHEDULED tasks must be included even if task_output is empty
19+
- SUCCESS tasks with succeeded = 0 must be excluded
20+
- FAILED / REVOKED tasks must be excluded
21+
"""
22+
23+
def setUp(self):
24+
super().setUp()
25+
self.initialize_course()
26+
self.instructor = self.create_instructor('instructor')
27+
28+
def test_includes_successful_bulk_email_task(self):
29+
"""
30+
SUCCESS + succeeded > 0 → INCLUDED
31+
"""
32+
task_output = json.dumps({
33+
"attempted": 10,
34+
"succeeded": 10,
35+
"failed": 0
36+
})
37+
38+
success_task = InstructorTaskFactory.create(
39+
course_id=self.course.id,
40+
task_type="bulk_course_email",
41+
task_id="bulk_email_success",
42+
task_input='{}',
43+
task_state=SUCCESS,
44+
task_output=task_output,
45+
task_key='bulk_email_success',
46+
requester=self.instructor
47+
)
48+
49+
tasks = list(get_instructor_task_history(
50+
self.course.id,
51+
task_type="bulk_course_email"
52+
))
53+
54+
assert success_task in tasks
55+
56+
def test_includes_scheduled_task_with_empty_output(self):
57+
"""
58+
SCHEDULED (even with empty {}) → INCLUDED
59+
"""
60+
scheduled_task = InstructorTaskFactory.create(
61+
course_id=self.course.id,
62+
task_type="bulk_course_email",
63+
task_id="bulk_email_scheduled",
64+
task_input='{}',
65+
task_state="SCHEDULED",
66+
task_output="{}",
67+
task_key='bulk_email_scheduled',
68+
requester=self.instructor
69+
)
70+
71+
tasks = list(get_instructor_task_history(
72+
self.course.id,
73+
task_type="bulk_course_email"
74+
))
75+
76+
assert scheduled_task in tasks
77+
78+
def test_excludes_zero_success_tasks(self):
79+
"""
80+
SUCCESS + succeeded = 0 → EXCLUDED
81+
"""
82+
zero_success_task = InstructorTaskFactory.create(
83+
course_id=self.course.id,
84+
task_type="bulk_course_email",
85+
task_id="bulk_email_zero",
86+
task_state=SUCCESS,
87+
task_output=json.dumps({
88+
"attempted": 10,
89+
"succeeded": 0,
90+
"failed": 10
91+
}),
92+
task_key='bulk_email_zero',
93+
requester=self.instructor
94+
)
95+
96+
tasks = list(get_instructor_task_history(
97+
self.course.id,
98+
task_type="bulk_course_email"
99+
))
100+
101+
assert zero_success_task not in tasks
102+
103+
def test_excludes_failed_tasks(self):
104+
"""
105+
FAILURE → EXCLUDED
106+
"""
107+
failed_task = InstructorTaskFactory.create(
108+
course_id=self.course.id,
109+
task_type="bulk_course_email",
110+
task_id="bulk_email_failed",
111+
task_state=FAILURE,
112+
task_output=json.dumps({
113+
"attempted": 5,
114+
"succeeded": 0,
115+
"failed": 5
116+
}),
117+
task_key='bulk_email_failed',
118+
requester=self.instructor
119+
)
120+
121+
tasks = list(get_instructor_task_history(
122+
self.course.id,
123+
task_type="bulk_course_email"
124+
))
125+
126+
assert failed_task not in tasks
127+
128+
def test_excludes_revoked_tasks(self):
129+
"""
130+
REVOKED → EXCLUDED
131+
"""
132+
revoked_task = InstructorTaskFactory.create(
133+
course_id=self.course.id,
134+
task_type="bulk_course_email",
135+
task_id="bulk_email_revoked",
136+
task_state=REVOKED,
137+
task_output='{"message": "Task revoked"}',
138+
task_key='bulk_email_revoked',
139+
requester=self.instructor
140+
)
141+
142+
tasks = list(get_instructor_task_history(
143+
self.course.id,
144+
task_type="bulk_course_email"
145+
))
146+
147+
assert revoked_task not in tasks
148+
149+
def test_only_valid_tasks_returned(self):
150+
"""
151+
Only the following should be returned:
152+
- SUCCESS with succeeded > 0
153+
- SCHEDULED
154+
155+
Everything else must be excluded.
156+
"""
157+
valid_success = InstructorTaskFactory.create(
158+
course_id=self.course.id,
159+
task_type="bulk_course_email",
160+
task_id="bulk_email_valid",
161+
task_state=SUCCESS,
162+
task_output=json.dumps({
163+
"attempted": 8,
164+
"succeeded": 5,
165+
"failed": 3
166+
}),
167+
task_key='bulk_email_valid',
168+
requester=self.instructor
169+
)
170+
171+
scheduled = InstructorTaskFactory.create(
172+
course_id=self.course.id,
173+
task_type="bulk_course_email",
174+
task_id="bulk_email_scheduled_2",
175+
task_state="SCHEDULED",
176+
task_output="{}",
177+
task_key='bulk_email_scheduled_2',
178+
requester=self.instructor
179+
)
180+
181+
zero_task = InstructorTaskFactory.create(
182+
course_id=self.course.id,
183+
task_type="bulk_course_email",
184+
task_id="bulk_email_zero_2",
185+
task_state=SUCCESS,
186+
task_output=json.dumps({
187+
"attempted": 5,
188+
"succeeded": 0,
189+
"failed": 5
190+
}),
191+
task_key='bulk_email_zero_2',
192+
requester=self.instructor
193+
)
194+
195+
tasks = list(get_instructor_task_history(
196+
self.course.id,
197+
task_type="bulk_course_email"
198+
))
199+
200+
assert valid_success in tasks
201+
assert scheduled in tasks
202+
assert zero_task not in tasks
203+
assert len(tasks) == 2

0 commit comments

Comments
 (0)