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.
This commit is contained in:
@@ -7,6 +7,7 @@ from django.dispatch import receiver
|
||||
from eventtracking import tracker
|
||||
|
||||
from common.djangoapps.student.models import CourseEnrollment
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS
|
||||
from edx_ace.signals import ACE_MESSAGE_SENT
|
||||
|
||||
@@ -27,7 +28,14 @@ def force_optout_all(sender, **kwargs): # lint-amnesty, pylint: disable=unused-
|
||||
raise TypeError('Expected a User type, but received None.')
|
||||
|
||||
for enrollment in CourseEnrollment.objects.filter(user=user):
|
||||
Optout.objects.get_or_create(user=user, course_id=enrollment.course.id)
|
||||
try:
|
||||
Optout.objects.get_or_create(user=user, course_id=enrollment.course.id)
|
||||
except CourseOverview.DoesNotExist:
|
||||
log.warning(
|
||||
f"CourseOverview not found for enrollment {enrollment.id} (user: {user.id}), "
|
||||
f"skipping optout creation. This may mean the course was deleted."
|
||||
)
|
||||
continue
|
||||
|
||||
|
||||
@receiver(ACE_MESSAGE_SENT)
|
||||
|
||||
@@ -10,9 +10,11 @@ from django.core import mail
|
||||
from django.core.management import call_command
|
||||
from django.urls import reverse
|
||||
|
||||
from common.djangoapps.student.models import CourseEnrollment
|
||||
from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
|
||||
from lms.djangoapps.bulk_email.models import BulkEmailFlag, Optout
|
||||
from lms.djangoapps.bulk_email.signals import force_optout_all
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
|
||||
|
||||
@@ -85,3 +87,41 @@ class TestOptoutCourseEmailsBySignal(ModuleStoreTestCase):
|
||||
assert len(mail.outbox) == 1
|
||||
assert len(mail.outbox[0].to) == 1
|
||||
assert mail.outbox[0].to[0] == self.instructor.email
|
||||
|
||||
@patch('lms.djangoapps.bulk_email.signals.log.warning')
|
||||
def test_optout_handles_missing_course_overview(self, mock_log_warning):
|
||||
"""
|
||||
Test that force_optout_all gracefully handles CourseEnrollments
|
||||
with missing CourseOverview records
|
||||
"""
|
||||
# Create a course key for a course that doesn't exist in CourseOverview
|
||||
nonexistent_course_key = CourseKey.from_string('course-v1:TestX+Missing+2023')
|
||||
|
||||
# Create an enrollment with a course_id that doesn't have a CourseOverview
|
||||
CourseEnrollment.objects.create(
|
||||
user=self.student,
|
||||
course_id=nonexistent_course_key,
|
||||
mode='honor'
|
||||
)
|
||||
|
||||
# Verify the orphaned enrollment exists
|
||||
assert CourseEnrollment.objects.filter(
|
||||
user=self.student,
|
||||
course_id=nonexistent_course_key
|
||||
).exists()
|
||||
|
||||
force_optout_all(sender=self.__class__, user=self.student)
|
||||
|
||||
# Verify that a warning was logged for the missing CourseOverview
|
||||
mock_log_warning.assert_called()
|
||||
call_args = mock_log_warning.call_args[0][0]
|
||||
assert "CourseOverview not found for enrollment" in call_args
|
||||
assert f"user: {self.student.id}" in call_args
|
||||
assert "skipping optout creation" in call_args
|
||||
|
||||
# Verify that optouts were created for valid courses only
|
||||
valid_course_optouts = Optout.objects.filter(user=self.student, course_id=self.course.id)
|
||||
missing_course_optouts = Optout.objects.filter(user=self.student, course_id=nonexistent_course_key)
|
||||
|
||||
assert valid_course_optouts.count() == 1
|
||||
assert missing_course_optouts.count() == 0
|
||||
|
||||
Reference in New Issue
Block a user