Merge pull request #15321 from edx/ri/EDUCATOR-394-disable-self-generated-certs

EDUCATOR-394 disable self-generated certificates when course is reset to instructor-paced

lms/djangoapps/certificates/signals.py

-""" Signal handler for enabling self-generated certificates by default
-for self-paced courses.
+"""
+Signal handler for enabling/disabling self-generated certificates based on the course-pacing.
 """
 from celery.task import task
-from django.dispatch.dispatcher import receiver
+from django.dispatch import receiver
 from opaque_keys.edx.keys import CourseKey
 
 from certificates.models import CertificateGenerationCourseSetting
-from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
-from xmodule.modulestore.django import SignalHandler
+from openedx.core.djangoapps.models.course_details import COURSE_PACING_CHANGE
 
 
-@receiver(SignalHandler.course_published)
-def _listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable=unused-argument
-    """ Catches the signal that a course has been published in Studio and
-    enable the self-generated certificates by default for self-paced
-    courses.
+@receiver(COURSE_PACING_CHANGE, dispatch_uid="course_pacing_changed")
+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.
     """
-    enable_self_generated_certs.delay(unicode(course_key))
+    toggle_self_generated_certs.delay(unicode(course_key), course_self_paced)
 
 
 @task()
-def enable_self_generated_certs(course_key):
-    """Enable the self-generated certificates by default for self-paced courses."""
+def toggle_self_generated_certs(course_key, course_self_paced):
+    """
+    Enable or disable self-generated certificates for a course according to pacing.
+    """
     course_key = CourseKey.from_string(course_key)
-    course = CourseOverview.get_from_id(course_key)
-    is_enabled_for_course = CertificateGenerationCourseSetting.is_enabled_for_course(course_key)
-    if course.self_paced and not is_enabled_for_course:
-        CertificateGenerationCourseSetting.set_enabled_for_course(course_key, True)
+    CertificateGenerationCourseSetting.set_enabled_for_course(course_key, course_self_paced)

lms/djangoapps/certificates/tests/test_signals.py

-""" Unit tests for enabling self-generated certificates by default
-for self-paced courses.
+"""
+Unit tests for enabling self-generated certificates for self-paced courses
+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
 
 
 class SelfGeneratedCertsSignalTest(ModuleStoreTestCase):
-    """ Tests for enabling self-generated certificates by default
-    for self-paced courses.
+    """
+    Tests for enabling/disabling self-generated certificates according to course-pacing.
     """
 
     def setUp(self):
@@ -21,11 +22,21 @@ class SelfGeneratedCertsSignalTest(ModuleStoreTestCase):
         # Enable the feature
         CertificateGenerationConfiguration.objects.create(enabled=True)
 
-    def test_cert_generation_enabled_for_self_paced(self):
-        """ Verify the signal enable the self-generated certificates by default for
-        self-paced courses.
+    def test_cert_generation_flag_on_pacing_toggle(self):
+        """
+        Verify that signal enables or disables self-generated certificates
+        according to course-pacing.
         """
+        #self-generation of cert disables by default
         self.assertFalse(certs_api.cert_generation_enabled(self.course.id))
 
-        _listen_for_course_publish('store', self.course.id)
+        _listen_for_course_pacing_changed('store', self.course.id, self.course.self_paced)
+        #verify that self-generation of cert is enabled for self-paced course
         self.assertTrue(certs_api.cert_generation_enabled(self.course.id))
+
+        self.course.self_paced = False
+        self.store.update_item(self.course, self.user.id)
+
+        _listen_for_course_pacing_changed('store', self.course.id, self.course.self_paced)
+        # verify that self-generation of cert is disabled for instructor-paced course
+        self.assertFalse(certs_api.cert_generation_enabled(self.course.id))

openedx/core/djangoapps/models/course_details.py

@@ -5,6 +5,7 @@ import re
 import logging
 
 from django.conf import settings
+from django.dispatch import Signal
 
 from xmodule.fields import Date
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -12,6 +13,8 @@ from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
 from openedx.core.lib.courses import course_image_url
 from xmodule.modulestore.django import modulestore
 
+COURSE_PACING_CHANGE = Signal(providing_args=["course_key", "course_self_paced"])
+
 
 # This list represents the attribute keys for a course's 'about' info.
 # Note: The 'video' attribute is intentionally excluded as it must be
@@ -188,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
@@ -271,10 +275,15 @@ class CourseDetails(object):
                 and jsondict['self_paced'] != descriptor.self_paced):
             descriptor.self_paced = jsondict['self_paced']
             dirty = True
+            is_pacing_changed = True
 
         if dirty:
             module_store.update_item(descriptor, user.id)
 
+        # fires a signal indicating that the course pacing has changed
+        if is_pacing_changed:
+            COURSE_PACING_CHANGE.send(sender=None, course_key=course_key, course_self_paced=descriptor.self_paced)
+
         # NOTE: below auto writes to the db w/o verifying that any of
         # the fields actually changed to make faster, could compare
         # against db or could have client send over a list of which