Review Rehan

lms/djangoapps/certificates/signals.py

@@ -10,7 +10,7 @@ from openedx.core.djangoapps.models.course_details import COURSE_PACING_CHANGE
 
 
 @receiver(COURSE_PACING_CHANGE, dispatch_uid="course_pacing_changed")
-def _listen_for_course_publish(sender, course_key, course_self_paced, **kwargs):  # pylint: disable=unused-argument
+def _listen_for_course_pacing_changed(sender, course_key, course_self_paced, **kwargs):  # pylint: disable=unused-argument
     """
     Catches the signal that course pacing has changed and enable/disable
     the self-generated certificates according to course-pacing.

lms/djangoapps/certificates/tests/test_signals.py

@@ -4,7 +4,7 @@ and disabling for instructor-paced courses.
 """
 from certificates import api as certs_api
 from certificates.models import CertificateGenerationConfiguration
-from certificates.signals import _listen_for_course_publish
+from certificates.signals import _listen_for_course_pacing_changed
 from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -22,23 +22,19 @@ class SelfGeneratedCertsSignalTest(ModuleStoreTestCase):
         # Enable the feature
         CertificateGenerationConfiguration.objects.create(enabled=True)
 
-    def test_cert_generation_enabled_for_self_paced(self):
+
+    def test_cert_generation_flag_on_pacing_toggle(self):
         """
-        Verify the signal enables the self-generated certificates for
-        self-paced courses.
+        Verify that signal enables or disables self-generated certificates
+        according to course-pacing.
         """
         self.assertFalse(certs_api.cert_generation_enabled(self.course.id))
 
-        _listen_for_course_publish('store', self.course.id, self.course.self_paced)
+        _listen_for_course_pacing_changed('store', self.course.id, self.course.self_paced)
         self.assertTrue(certs_api.cert_generation_enabled(self.course.id))
 
-    def test_cert_generation_disabled_for_instructor_paced(self):
-        """
-        Verify the signal disables the self-generated certificates for
-        instructor-paced courses.
-        """
         self.course.self_paced = False
         self.store.update_item(self.course, self.user.id)
 
-        _listen_for_course_publish('store', self.course.id, self.course.self_paced)
+        _listen_for_course_pacing_changed('store', self.course.id, self.course.self_paced)
         self.assertFalse(certs_api.cert_generation_enabled(self.course.id))

openedx/core/djangoapps/models/course_details.py

@@ -191,6 +191,7 @@ class CourseDetails(object):
         descriptor = module_store.get_course(course_key)
 
         dirty = False
+        is_pacing_changed = False
 
         # In the descriptor's setter, the date is converted to JSON
         # using Date's to_json method. Calling to_json on something that
@@ -274,8 +275,7 @@ class CourseDetails(object):
                 and jsondict['self_paced'] != descriptor.self_paced):
             descriptor.self_paced = jsondict['self_paced']
             dirty = True
-            # sends out the signal that course pacing has changed
-            COURSE_PACING_CHANGE.send(sender=None, course_key=course_key, course_self_paced=descriptor.self_paced)
+            is_pacing_changed = True
 
         if dirty:
             module_store.update_item(descriptor, user.id)
@@ -290,6 +290,10 @@ class CourseDetails(object):
 
         cls.update_about_video(descriptor, jsondict['intro_video'], user.id)
 
+        # fires the signal that course pacing has changed after changes are reflected in db
+        if is_pacing_changed:
+            COURSE_PACING_CHANGE.send(sender=None, course_key=course_key, course_self_paced=descriptor.self_paced)
+
         # Could just return jsondict w/o doing any db reads, but I put
         # the reads in as a means to confirm it persisted correctly
         return CourseDetails.fetch(course_key)