fix: fix and refactor task annd signals responsible for syncing cert …

…available date updates to Credentials

[APER-3229]

The monolith and the Credentials IDA keep independent records of a course runs certificate availability/visibility preferences. This PR aims to improve the communication between the monolith and the Credentiala IDA to keep the availability date/preference in sync with the monoliths records.

The current code is too strict and actually prevents valid updates in some configurations.

Additionally, the Credentials IDA doesn't understand the concept of "course pacing" (instructor-paced vs self-paced) and has troubles with courses with an availability date of "end". Instead of having to add the concept of course pacing (and syncing more data between the two systems), this PR proposes sending the end date of a course as the "certificate available date" to Credentials.

This way, the Credentials IDA can manage the visibility of awarded credentials in a course run with a display behavior of "end" using the existing feature set and models of the Credentials service.

openedx/core/djangoapps/content/course_overviews/signals.py

@@ -11,7 +11,8 @@
 from django.dispatch.dispatcher import receiver
 
 from openedx.core.djangoapps.signals.signals import COURSE_CERT_DATE_CHANGE
-from xmodule.modulestore.django import SignalHandler  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.data import CertificatesDisplayBehaviors
+from xmodule.modulestore.django import SignalHandler
 
 from .models import CourseOverview
 
@@ -30,8 +31,8 @@
 @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
-    updates the corresponding CourseOverview cache entry.
+    Catches the signal that a course has been published in Studio and updates the corresponding CourseOverview cache
+    entry.
     """
     try:
         previous_course_overview = CourseOverview.objects.get(id=course_key)
@@ -72,13 +73,37 @@ def trigger_import_course_details_signal(sender, instance, created, **kwargs):
 
 
 def _check_for_course_changes(previous_course_overview, updated_course_overview):
+    """
+    Utility function responsible for calling other utility functions that check for specific changes in a course
+    overview after a course run has been updated and published.
+
+    Args:
+        previous_course_overview (CourseOverview): the current course overview instance for a particular course run
+        updated_course_overview (CourseOverview): an updated course overview instance, reflecting the current state of
+        data from the modulestore/Mongo
+
+    Returns:
+        None
+    """
     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):
+    """
+    Checks if a course run's start date has been updated. If so, we emit the `COURSE_START_DATE_CHANGED` signal to
+    ensure other parts of the system are aware of the change.
+
+    Args:
+        previous_course_overview (CourseOverview): the current course overview instance for a particular course run
+        updated_course_overview (CourseOverview): an updated course overview instance, reflecting the current state of
+        data from the modulestore/Mongo
+
+    Returns:
+        None
+    """
     if previous_course_overview.start != updated_course_overview.start:
         _log_start_date_change(previous_course_overview, updated_course_overview)
         COURSE_START_DATE_CHANGED.send(
@@ -88,21 +113,46 @@ def _check_for_course_date_changes(previous_course_overview, updated_course_over
         )
 
 
-def _log_start_date_change(previous_course_overview, updated_course_overview):  # lint-amnesty, pylint: disable=missing-function-docstring
+def _log_start_date_change(previous_course_overview, updated_course_overview):
+    """
+    Utility function to log a course run's start date when updating a course overview. This log only appears when the
+    start date has been changed (see the `_check_for_course_date_changes` function above).
+
+    Args:
+        previous_course_overview (CourseOverview): the current course overview instance for a particular course run
+        updated_course_overview (CourseOverview): an updated course overview instance, reflecting the current state of
+        data from the modulestore/Mongo
+
+    Returns:
+        None
+    """
     previous_start_str = 'None'
     if previous_course_overview.start is not None:
         previous_start_str = previous_course_overview.start.isoformat()
     new_start_str = 'None'
     if updated_course_overview.start is not None:
         new_start_str = updated_course_overview.start.isoformat()
-    LOG.info('Course start date changed: course={} previous={} new={}'.format(
-        updated_course_overview.id,
-        previous_start_str,
-        new_start_str,
-    ))
+    LOG.info(
+        f"Course start date changed: course={updated_course_overview.id} previous={previous_start_str} "
+        f"new={new_start_str}"
+    )
 
 
 def _check_for_pacing_changes(previous_course_overview, updated_course_overview):
+    """
+    Checks if a course run's pacing has been updated. If so, we emit the `COURSE_PACING_CHANGED` signal to ensure other
+    parts of the system are aware of the change. The `programs` and `certificates` apps listen for this signal in
+    order to manage certificate generation features in the LMS and certificate visibility settings in the Credentials
+    IDA.
+
+    Args:
+        previous_course_overview (CourseOverview): the current course overview instance for a particular course run
+        updated_course_overview (CourseOverview): an updated course overview instance, reflecting the current state of
+        data from the modulestore/Mongo
+
+    Returns:
+        None
+    """
     if previous_course_overview.self_paced != updated_course_overview.self_paced:
         COURSE_PACING_CHANGED.send(
             sender=None,
@@ -112,18 +162,60 @@ def _check_for_pacing_changes(previous_course_overview, updated_course_overview)
 
 
 def _check_for_cert_availability_date_changes(previous_course_overview, updated_course_overview):
-    """ Checks if the cert available date has changed and if so, sends a COURSE_CERT_DATE_CHANGE signal"""
-    if previous_course_overview.certificate_available_date != updated_course_overview.certificate_available_date:
+    """
+    Checks if the certificate available date (CAD) or the certificates display behavior (CDB) of a course run has
+    changed during a course overview update. If so, we emit the COURSE_CERT_DATE_CHANGE signal to ensure other parts of
+    the system are aware of the change. The `credentials` app listens for this signal in order to keep our certificate
+    visibility settings in the Credentials IDA up to date.
+
+    Args:
+        previous_course_overview (CourseOverview): the current course overview instance for a particular course run
+        updated_course_overview (CourseOverview): an updated course overview instance, reflecting the current state of
+            data from the modulestore/Mongo
+
+    Returns:
+        None
+    """
+    def _send_course_cert_date_change_signal():
+        """
+        A callback used to fire the COURSE_CERT_DATE_CHANGE Django signal *after* the ORM has successfully commit the
+        update.
+        """
+        COURSE_CERT_DATE_CHANGE.send_robust(sender=None, course_key=str(updated_course_overview.id))
+
+    course_run_id = str(updated_course_overview.id)
+    prev_available_date = previous_course_overview.certificate_available_date
+    prev_display_behavior = previous_course_overview.certificates_display_behavior
+    prev_end_date = previous_course_overview.end  # `end_date` is a deprecated field, use `end` instead
+    updated_available_date = updated_course_overview.certificate_available_date
+    updated_display_behavior = updated_course_overview.certificates_display_behavior
+    updated_end_date = updated_course_overview.end  # `end_date` is a deprecated field, use `end` instead
+
+    send_signal = False
+    if prev_available_date != updated_available_date:
         LOG.info(
-            f"Certificate availability date for {str(updated_course_overview.id)} has changed from " +
-            f"{previous_course_overview.certificate_available_date} to " +
-            f"{updated_course_overview.certificate_available_date}. Sending COURSE_CERT_DATE_CHANGE signal."
+            f"The certificate available date for {course_run_id} has changed from {prev_available_date} to "
+            f"{updated_available_date}. Firing COURSE_CERT_DATE_CHANGE signal."
         )
+        send_signal = True
+    elif prev_display_behavior != updated_display_behavior:
+        LOG.info(
+            f"The certificates display behavior for {course_run_id} has changed from {prev_display_behavior} to "
+            f"{updated_display_behavior}. Firing COURSE_CERT_DATE_CHANGE signal."
+        )
+        send_signal = True
+    # edge case -- if a course run with a cert display behavior of "End date of course" has changed its end date, we
+    # should fire our signal to ensure visibility of certificates managed by the Credentials IDA are corrected too
+    elif (
+        (prev_display_behavior == CertificatesDisplayBehaviors.END and
+         updated_display_behavior == CertificatesDisplayBehaviors.END) and
+        (prev_end_date != updated_end_date)
+    ):
+        LOG.info(
+            f"The end date for {course_run_id} has changed from {prev_end_date} to {updated_end_date}. Firing "
+            "COURSE_CERT_DATE_CHANGE signal."
+        )
+        send_signal = True
 
-        def _send_course_cert_date_change_signal():
-            COURSE_CERT_DATE_CHANGE.send_robust(
-                sender=None,
-                course_key=updated_course_overview.id,
-            )
-
+    if send_signal:
         transaction.on_commit(_send_course_cert_date_change_signal)

openedx/core/djangoapps/content/course_overviews/tests/test_signals.py

@@ -1,4 +1,7 @@
-# lint-amnesty, pylint: disable=missing-module-docstring
+"""
+Tests for the course_overviews app's signal functionality.
+"""
+
 
 import datetime
 from unittest.mock import patch
@@ -29,13 +32,32 @@ class CourseOverviewSignalsTestCase(ModuleStoreTestCase):
     TODAY = datetime.datetime.utcnow().replace(tzinfo=UTC)
     NEXT_WEEK = TODAY + datetime.timedelta(days=7)
 
+    def assert_changed_signal_sent(self, changes, mock_signal):
+        """
+        Utility function used to verify that an emulated change to a course overview results in the expected signals
+        being fired by the system.
+        """
+        course = CourseFactory.create(
+            emit_signals=True,
+            **{change.field_name: change.initial_value for change in changes}
+        )
+
+        # changing display name doesn't fire the signal
+        with self.captureOnCommitCallbacks(execute=True) as callbacks:
+            course.display_name = course.display_name + 'changed'
+            course = self.store.update_item(course, ModuleStoreEnum.UserID.test)
+        assert not mock_signal.called
+
+        # changing the given field fires the signal
+        with self.captureOnCommitCallbacks(execute=True) as callbacks:
+            for change in changes:
+                setattr(course, change.field_name, change.changed_value)
+            self.store.update_item(course, ModuleStoreEnum.UserID.test)
+        assert mock_signal.called
+
     def test_caching(self):
         """
         Tests that CourseOverview structures are actually getting cached.
-
-        Arguments:
-            modulestore_type (ModuleStoreEnum.Type): type of store to create the
-                course in.
         """
         # Creating a new course will trigger a publish event and the course will be cached
         course = CourseFactory.create(emit_signals=True)
@@ -46,12 +68,7 @@ def test_caching(self):
 
     def test_cache_invalidation(self):
         """
-        Tests that when a course is published or deleted, the corresponding
-        course_overview is removed from the cache.
-
-        Arguments:
-            modulestore_type (ModuleStoreEnum.Type): type of store to create the
-                course in.
+        Tests that when a course is published or deleted, the corresponding course_overview is removed from the cache.
         """
         # Create a course where mobile_available is True.
         course = CourseFactory.create(mobile_available=True)
@@ -74,31 +91,18 @@ def test_cache_invalidation(self):
             self.store.delete_course(course.id, ModuleStoreEnum.UserID.test)
             CourseOverview.get_from_id(course.id)
 
-    def assert_changed_signal_sent(self, changes, mock_signal):  # lint-amnesty, pylint: disable=missing-function-docstring
-        course = CourseFactory.create(
-            emit_signals=True,
-            **{change.field_name: change.initial_value for change in changes}
-        )
-
-        # changing display name doesn't fire the signal
-        with self.captureOnCommitCallbacks(execute=True) as callbacks:
-            course.display_name = course.display_name + 'changed'
-            course = self.store.update_item(course, ModuleStoreEnum.UserID.test)
-        assert not mock_signal.called
-
-        # changing the given field fires the signal
-        with self.captureOnCommitCallbacks(execute=True) as callbacks:
-            for change in changes:
-                setattr(course, change.field_name, change.changed_value)
-            self.store.update_item(course, ModuleStoreEnum.UserID.test)
-        assert mock_signal.called
-
     @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_START_DATE_CHANGED.send')
     def test_start_changed(self, mock_signal):
+        """
+        A test that ensures the `COURSE_STATE_DATE_CHANGED` signal is emit when the start date of course run is updated.
+        """
         self.assert_changed_signal_sent([Change('start', self.TODAY, self.NEXT_WEEK)], mock_signal)
 
     @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_PACING_CHANGED.send')
     def test_pacing_changed(self, mock_signal):
+        """
+        A test that ensures the `COURSE_PACING_CHANGED` signal is emit when the pacing type of a course run is updated.
+        """
         self.assert_changed_signal_sent([Change('self_paced', True, False)], mock_signal)
 
     @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_CERT_DATE_CHANGE.send_robust')
@@ -112,3 +116,21 @@ def test_cert_date_changed(self, mock_signal):
             )
         ]
         self.assert_changed_signal_sent(changes, mock_signal)
+
+    @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_CERT_DATE_CHANGE.send_robust')
+    def test_cert_end_date_changed(self, mock_signal):
+        """
+        This test ensures when an instrucor-paced course with a certificates display behavior of "END" updates its end
+        date that we emit the `COURSE_CERT_DATE_CHANGE` signal.
+        """
+        course = CourseFactory.create(
+            emit_signals=True,
+            end=self.TODAY,
+            certificates_display_behavior=CertificatesDisplayBehaviors.END
+        )
+
+        with self.captureOnCommitCallbacks(execute=True):
+            setattr(course, "end", self.NEXT_WEEK)
+            self.store.update_item(course, ModuleStoreEnum.UserID.test)
+
+        assert mock_signal.called

openedx/core/djangoapps/credentials/api.py

@@ -0,0 +1,15 @@
+"""
+Python APIs exposed by the credentials app to other in-process apps.
+"""
+
+
+from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
+
+
+def is_credentials_enabled():
+    """
+    A utility function wrapping the `is_learner_issurance_enabled` utility function of the CredentialsApiConfig model.
+    Intended to be an easier to read/grok utility function that informs the caller if use of the Credentials IDA is
+    enabled for this Open edX instance.
+    """
+    return CredentialsApiConfig.current().is_learner_issuance_enabled

openedx/core/djangoapps/credentials/tests/test_api.py

@@ -0,0 +1,46 @@
+"""
+Tests for the utility functions defined as part of the credentials app's public Python API.
+"""
+
+
+from django.test import TestCase
+
+from openedx.core.djangoapps.credentials.api import is_credentials_enabled
+from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
+from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
+
+
+class CredentialsApiTests(CredentialsApiConfigMixin, TestCase):
+    """
+    Tests for the Public Pyton API exposed by the credentials Django app.
+    """
+
+    def setUp(self):
+        super().setUp()
+
+    def tearDown(self):
+        CredentialsApiConfig.objects.all().delete()
+        super().tearDown()
+
+    def test_is_credentials_enabled_config_enabled(self):
+        """
+        A test that verifies the output of the `is_credentials_enabled` utility function when a CredentialsApiConfig
+        exists and is enabled.
+        """
+        self.create_credentials_config(enabled=True)
+        assert is_credentials_enabled()
+
+    def test_is_credentials_enabled_config_disabled(self):
+        """
+        A test that verifies the output of the `is_credentials_enabled` utility function when a CredentialsApiConfig
+        exists and is disabled.
+        """
+        self.create_credentials_config(enabled=False)
+        assert not is_credentials_enabled()
+
+    def test_is_credentials_enabled_config_absent(self):
+        """
+        A test that verifies the output of the `is_credentials_enabled` utility function when a CredentialsApiConfig
+        does not exist.
+        """
+        assert not is_credentials_enabled()