Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Move cert date signals to avoid race conditions

Browse Source 
COURSE_CERT_DATE_CHANGE was being called before saving the new data in
the course overview. The listeners were expecting to pull the data out
of the course overview, and thus were only right about half the time.
This moves the signal to trigger after the course publish signals are
handled.
This commit is contained in:
Matt Tuchfarber
2021-03-03 17:32:47 -05:00
parent 2b4b53d36f
commit 6c97dfe1e5
7 changed files with 30 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
openedx/core/djangoapps/content/course_overviews/signals.py
View File

@@ -8,6 +8,7 @@ import logging
from django.dispatch import Signal
from django.dispatch.dispatcher import receiver
from openedx.core.djangoapps.signals.signals import COURSE_CERT_DATE_CHANGE
from xmodule.modulestore.django import SignalHandler
from .models import CourseOverview
@@ -50,6 +51,7 @@ def _check_for_course_changes(previous_course_overview, updated_course_overview)
if previous_course_overview:
_check_for_course_date_changes(previous_course_overview, updated_course_overview)
_check_for_pacing_changes(previous_course_overview, updated_course_overview)
_check_for_cert_availability_date_changes(previous_course_overview, updated_course_overview)
def _check_for_course_date_changes(previous_course_overview, updated_course_overview):
@@ -83,3 +85,8 @@ def _check_for_pacing_changes(previous_course_overview, updated_course_overview)
updated_course_overview=updated_course_overview,
previous_self_paced=previous_course_overview.self_paced,
)
def _check_for_cert_availability_date_changes(previous_course_overview, updated_course_overview):
if previous_course_overview.certificate_available_date != updated_course_overview.certificate_available_date:
COURSE_CERT_DATE_CHANGE.send_robust(sender=None, course_key=updated_course_overview.id)

4
openedx/core/djangoapps/content/course_overviews/tests/test_signals.py
View File

@@ -90,3 +90,7 @@ class CourseOverviewSignalsTestCase(ModuleStoreTestCase):
@patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_PACING_CHANGED.send')
def test_pacing_changed(self, mock_signal):
self.assert_changed_signal_sent('self_paced', True, False, mock_signal)
@patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_CERT_DATE_CHANGE.send_robust')
def test_cert_date_changed(self, mock_signal):
self.assert_changed_signal_sent('certificate_available_date', self.TODAY, self.NEXT_WEEK, mock_signal)

1
openedx/core/djangoapps/courseware_api/views.py
View File

@@ -6,7 +6,6 @@ import json
from completion.exceptions import UnavailableCompletionData
from completion.utilities import get_key_to_last_completed_block
from django.conf import settings
from django.urls import reverse
from django.utils.translation import ugettext as _
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication

5
openedx/core/djangoapps/models/course_details.py
View File

@@ -6,11 +6,8 @@ CourseDetails
import logging
import re
import six
from django.conf import settings
from openedx.core.djangoapps.models.config.waffle import enable_course_detail_update_certificate_date
from openedx.core.djangoapps.signals.signals import COURSE_CERT_DATE_CHANGE
from openedx.core.djangolib.markup import HTML
from openedx.core.lib.courses import course_image_url
from xmodule.fields import Date
@@ -246,8 +243,6 @@ class CourseDetails(object):
if converted != descriptor.certificate_available_date:
dirty = True
descriptor.certificate_available_date = converted
if enable_course_detail_update_certificate_date(course_key):
COURSE_CERT_DATE_CHANGE.send_robust(sender=cls, course_key=six.text_type(course_key))
if 'course_image_name' in jsondict and jsondict['course_image_name'] != descriptor.course_image:
descriptor.course_image = jsondict['course_image_name']

11
openedx/core/djangoapps/programs/signals.py
View File

@@ -8,6 +8,7 @@ import logging
from django.dispatch import receiver
from openedx.core.djangoapps.credentials.helpers import is_learner_records_enabled_for_org
from openedx.core.djangoapps.models.config.waffle import enable_course_detail_update_certificate_date
from openedx.core.djangoapps.signals.signals import (
COURSE_CERT_AWARDED,
COURSE_CERT_CHANGED,
@@ -82,8 +83,7 @@ def handle_course_cert_changed(sender, user, course_key, mode, status, **kwargs)
Returns:
None
"""
"""
# Import here instead of top of file since this module gets imported before
# the credentials app is loaded, resulting in a Django deprecation warning.
from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
@@ -196,6 +196,10 @@ def handle_course_cert_date_change(sender, course_key, **kwargs): # lint-amnest
None
"""
# Stop if cert date updating isn't in effect for the course
if not enable_course_detail_update_certificate_date(course_key):
return
# Import here instead of top of file since this module gets imported before
# the credentials app is loaded, resulting in a Django deprecation warning.
from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
@@ -209,6 +213,7 @@ def handle_course_cert_date_change(sender, course_key, **kwargs): # lint-amnest
'handling COURSE_CERT_DATE_CHANGE for course %s',
course_key,
)
# import here, because signal is registered at startup, but items in tasks are not yet loaded
from openedx.core.djangoapps.programs.tasks import update_certificate_visible_date_on_course_update
update_certificate_visible_date_on_course_update.delay(course_key)
update_certificate_visible_date_on_course_update.delay(str(course_key))

5
openedx/core/djangoapps/programs/tasks.py
View File

@@ -606,7 +606,6 @@ def update_certificate_visible_date_on_course_update(self, course_key):
# feature, it may indicate a condition where processing of such tasks
# has been temporarily disabled. Since this is a recoverable situation,
# mark this task for retry instead of failing it altogether.
if not CredentialsApiConfig.current().is_learner_issuance_enabled:
error_msg = (
"Task update_certificate_visible_date_on_course_update cannot be executed when credentials issuance is "
@@ -622,5 +621,9 @@ def update_certificate_visible_date_on_course_update(self, course_key):
course_id=course_key
).values_list('user__username', flat=True)
LOGGER.info(
"Task update_certificate_visible_date_on_course_update resending course certificates"
f"for {len(users_with_certificates_in_course)} users in course {course_key}."
)
for user in users_with_certificates_in_course:
award_course_certificate.delay(user, str(course_key))

10
openedx/core/djangoapps/programs/tests/test_signals.py
View File

@@ -238,6 +238,7 @@ class CertRevokedReceiverTest(TestCase):
new_callable=mock.PropertyMock,
return_value=False,
)
@mock.patch("openedx.core.djangoapps.programs.signals.enable_course_detail_update_certificate_date")
class CourseCertAvailableDateChangedReceiverTest(TestCase):
"""
Tests for the `handle_course_cert_date_change` signal handler function.
@@ -253,7 +254,7 @@ class CourseCertAvailableDateChangedReceiverTest(TestCase):
'course_key': TEST_COURSE_KEY,
}
def test_signal_received(self, mock_is_learner_issuance_enabled, mock_task): # pylint: disable=unused-argument
def test_signal_received(self, mock_enable_update, mock_is_learner_issuance_enabled, mock_task): # pylint: disable=unused-argument
"""
Ensures the receiver function is invoked when COURSE_CERT_DATE_CHANGE is
sent.
@@ -262,23 +263,26 @@ class CourseCertAvailableDateChangedReceiverTest(TestCase):
to the way django signals work), we mock a configuration call that is
known to take place inside the function.
"""
mock_enable_update.return_value = True
COURSE_CERT_DATE_CHANGE.send(**self.signal_kwargs)
assert mock_is_learner_issuance_enabled.call_count == 1
def test_programs_disabled(self, mock_is_learner_issuance_enabled, mock_task):
def test_programs_disabled(self, mock_enable_update, mock_is_learner_issuance_enabled, mock_task):
"""
Ensures that the receiver function does nothing when the credentials API
configuration is not enabled.
"""
mock_enable_update.return_value = True
handle_course_cert_date_change(**self.signal_kwargs)
assert mock_is_learner_issuance_enabled.call_count == 1
assert mock_task.call_count == 0
def test_programs_enabled(self, mock_is_learner_issuance_enabled, mock_task):
def test_programs_enabled(self, mock_enable_update, mock_is_learner_issuance_enabled, mock_task):
"""
Ensures that the receiver function invokes the expected celery task
when the credentials API configuration is enabled.
"""
mock_enable_update.return_value = True
mock_is_learner_issuance_enabled.return_value = True
handle_course_cert_date_change(**self.signal_kwargs)