Skip to content

Commit 50da280

Browse files
authored
fix: handle CourseOverview.DoesNotExist exception in optout creation (#77) (#37885)
User retirement failed when CourseOverview records were missing, causing CourseOverview.DoesNotExist exceptions in bulk email signal handler. These records were missing when the course was deleted. Solution: Add exception handling in force_optout_all signal handler: - Wrapped Optout.objects.get_or_create() in try/except block - Log warning and skip enrollment when CourseOverview is missing.
1 parent c9704c2 commit 50da280

File tree

2 files changed

+49
-1
lines changed

2 files changed

+49
-1
lines changed

lms/djangoapps/bulk_email/signals.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from eventtracking import tracker
88

99
from common.djangoapps.student.models import CourseEnrollment
10+
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
1011
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS
1112
from edx_ace.signals import ACE_MESSAGE_SENT
1213

@@ -27,7 +28,14 @@ def force_optout_all(sender, **kwargs): # lint-amnesty, pylint: disable=unused-
2728
raise TypeError('Expected a User type, but received None.')
2829

2930
for enrollment in CourseEnrollment.objects.filter(user=user):
30-
Optout.objects.get_or_create(user=user, course_id=enrollment.course.id)
31+
try:
32+
Optout.objects.get_or_create(user=user, course_id=enrollment.course.id)
33+
except CourseOverview.DoesNotExist:
34+
log.warning(
35+
f"CourseOverview not found for enrollment {enrollment.id} (user: {user.id}), "
36+
f"skipping optout creation. This may mean the course was deleted."
37+
)
38+
continue
3139

3240

3341
@receiver(ACE_MESSAGE_SENT)

lms/djangoapps/bulk_email/tests/test_signals.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@
1010
from django.core.management import call_command
1111
from django.urls import reverse
1212

13+
from common.djangoapps.student.models import CourseEnrollment
1314
from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
1415
from lms.djangoapps.bulk_email.models import BulkEmailFlag, Optout
1516
from lms.djangoapps.bulk_email.signals import force_optout_all
17+
from opaque_keys.edx.keys import CourseKey
1618
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
1719
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
1820

@@ -85,3 +87,41 @@ def test_optout_course(self):
8587
assert len(mail.outbox) == 1
8688
assert len(mail.outbox[0].to) == 1
8789
assert mail.outbox[0].to[0] == self.instructor.email
90+
91+
@patch('lms.djangoapps.bulk_email.signals.log.warning')
92+
def test_optout_handles_missing_course_overview(self, mock_log_warning):
93+
"""
94+
Test that force_optout_all gracefully handles CourseEnrollments
95+
with missing CourseOverview records
96+
"""
97+
# Create a course key for a course that doesn't exist in CourseOverview
98+
nonexistent_course_key = CourseKey.from_string('course-v1:TestX+Missing+2023')
99+
100+
# Create an enrollment with a course_id that doesn't have a CourseOverview
101+
CourseEnrollment.objects.create(
102+
user=self.student,
103+
course_id=nonexistent_course_key,
104+
mode='honor'
105+
)
106+
107+
# Verify the orphaned enrollment exists
108+
assert CourseEnrollment.objects.filter(
109+
user=self.student,
110+
course_id=nonexistent_course_key
111+
).exists()
112+
113+
force_optout_all(sender=self.__class__, user=self.student)
114+
115+
# Verify that a warning was logged for the missing CourseOverview
116+
mock_log_warning.assert_called()
117+
call_args = mock_log_warning.call_args[0][0]
118+
assert "CourseOverview not found for enrollment" in call_args
119+
assert f"user: {self.student.id}" in call_args
120+
assert "skipping optout creation" in call_args
121+
122+
# Verify that optouts were created for valid courses only
123+
valid_course_optouts = Optout.objects.filter(user=self.student, course_id=self.course.id)
124+
missing_course_optouts = Optout.objects.filter(user=self.student, course_id=nonexistent_course_key)
125+
126+
assert valid_course_optouts.count() == 1
127+
assert missing_course_optouts.count() == 0

0 commit comments

Comments
 (0)