feat: Emit COURSE_CATALOG_INFO_CHANGED signal on course publish

Implements #30682 Produce signal only once transaction for a course publish is committed, and only for actual courses (not libraries). - Use newer openedx-events version that has a fix for None datetime and that has CourseCatalogData without org, number. - Add edx-event-bus-kafka -- specify recent version that drops confluent-kafka from explicit deps - New functionality behind toggle As per openedx/openedx-events#88 we're going to try explicit dependencies on implementations for now, rather than solve all the problems we'd encounter by using private dependencies.
openedx · Aug 23, 2022 · 4c2a753 · 4c2a753
1 parent fe8a8ef
commit 4c2a753
Show file tree

Hide file tree

Showing 9 changed files with 212 additions and 8 deletions.
diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py
@@ -4,10 +4,17 @@
 import logging
 from datetime import datetime
 from functools import wraps
+from typing import Optional
 
 from django.conf import settings
 from django.core.cache import cache
+from django.db import transaction
 from django.dispatch import receiver
+from edx_toggles.toggles import SettingToggle
+from edx_event_bus_kafka.publishing.event_producer import send_to_event_bus
+from opaque_keys.edx.keys import CourseKey
+from openedx_events.content_authoring.data import CourseCatalogData, CourseScheduleData
+from openedx_events.content_authoring.signals import COURSE_CATALOG_INFO_CHANGED
 from pytz import UTC
 
 from cms.djangoapps.contentstore.courseware_index import (
@@ -20,8 +27,10 @@
 from lms.djangoapps.grades.api import task_compute_all_grades_for_course
 from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines
 from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task
+from openedx.core.djangoapps.models.course_details import CourseDetails
 from openedx.core.lib.gating import api as gating_api
-from xmodule.modulestore.django import SignalHandler, modulestore  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import SignalHandler, modulestore
 from .signals import GRADING_POLICY_CHANGED
 
 log = logging.getLogger(__name__)
@@ -43,6 +52,65 @@ def wrapper(*args, **kwargs):
     return task_decorator
 
 
+# .. toggle_name: SEND_CATALOG_INFO_SIGNAL
+# .. toggle_implementation: SettingToggle
+# .. toggle_default: False
+# .. toggle_description: When True, sends to catalog-info-changed signal when course_published occurs.
+#   This is a temporary toggle to allow us to test the event bus integration; it should be removed and
+#   always-on once the integration is well-tested and the error cases are handled. (This is separate
+#   from whether the event bus itself is configured; if this toggle is on but the event bus is not
+#   configured, we should expect a warning at most.)
+# .. toggle_use_cases: temporary
+# .. toggle_creation_date: 2022-08-22
+# .. toggle_target_removal_date: 2022-10-30
+# .. toggle_tickets: https://github.com/openedx/edx-platform/issues/30682
+SEND_CATALOG_INFO_SIGNAL = SettingToggle('SEND_CATALOG_INFO_SIGNAL', default=False, module_name=__name__)
+
+
+def create_catalog_data_for_signal(course_key: CourseKey) -> Optional[CourseCatalogData]:
+    """
+    Creates data for catalog-info-changed signal when course is published.
+
+    Arguments:
+        course_key: Key of the course to announce catalog info changes for
+
+    Returns:
+        Data for signal, or None if not appropriate to send on this signal.
+    """
+    # Only operate on real courses, not libraries.
+    if not course_key.is_course:
+        return None
+
+    store = modulestore()
+    with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key):
+        course = store.get_course(course_key)
+
+        return CourseCatalogData(
+            course_key=course_key.for_branch(None),  # Shouldn't be necessary, but just in case...
+            name=course.display_name,
+            schedule_data=CourseScheduleData(
+                start=course.start,
+                pacing='self' if course.self_paced else 'instructor',
+                end=course.end,
+                enrollment_start=course.enrollment_start,
+                enrollment_end=course.enrollment_end,
+            ),
+            effort=CourseDetails.fetch_about_attribute(course_key, 'effort'),
+            hidden=course.catalog_visibility in ['about', 'none'] or course_key.deprecated,
+            invitation_only=course.invitation_only,
+        )
+
+
+def emit_catalog_info_changed_signal(course_key: CourseKey):
+    """
+    Given the key of a recently published course, send course data to catalog-info-changed signal.
+    """
+    if SEND_CATALOG_INFO_SIGNAL.is_enabled():
+        catalog_info = create_catalog_data_for_signal(course_key)
+        if catalog_info is not None:
+            COURSE_CATALOG_INFO_CHANGED.send_event(catalog_info=catalog_info)
+
+
 @receiver(SignalHandler.course_published)
 def listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable=unused-argument
     """
@@ -79,6 +147,20 @@ def listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable=
 
     update_discussions_settings_from_course_task.delay(course_key_str)
 
+    # Send to a signal for catalog info changes as well, but only once we know the transaction is committed.
+    transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key))
+
+
+@receiver(COURSE_CATALOG_INFO_CHANGED)
+def listen_for_course_catalog_info_changed(sender, signal, **kwargs):
+    """
+    Publish COURSE_CATALOG_INFO_CHANGED signals onto the event bus.
+    """
+    send_to_event_bus(
+        signal=COURSE_CATALOG_INFO_CHANGED, topic='course-catalog-info-changed',
+        event_key_field='catalog_info.course_key', event_data={'catalog_info': kwargs['catalog_info']}
+    )
+
 
 @receiver(SignalHandler.course_deleted)
 def listen_for_course_delete(sender, course_key, **kwargs):  # pylint: disable=unused-argument

diff --git a/cms/djangoapps/contentstore/signals/tests/__init__.py b/cms/djangoapps/contentstore/signals/tests/__init__.py
diff --git a/cms/djangoapps/contentstore/signals/tests/test_handlers.py b/cms/djangoapps/contentstore/signals/tests/test_handlers.py
@@ -0,0 +1,83 @@
+"""
+Tests for signal handlers in the contentstore.
+"""
+
+from datetime import datetime
+from unittest.mock import patch
+
+from django.test.utils import override_settings
+from opaque_keys.edx.locator import CourseLocator, LibraryLocator
+from openedx_events.content_authoring.data import CourseCatalogData, CourseScheduleData
+
+import cms.djangoapps.contentstore.signals.handlers as sh
+from xmodule.modulestore.django import SignalHandler
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import SampleCourseFactory
+
+
+class TestCatalogInfoSignal(ModuleStoreTestCase):
+    """
+    Test functionality of triggering catalog info signals (and events) from course_published signal.
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.course = SampleCourseFactory.create(
+            org='TestU',
+            number='sig101',
+            display_name='Signals 101',
+            run='Summer2022',
+        )
+        self.course_key = self.course.id
+
+        self.expected_data = CourseCatalogData(
+            course_key=CourseLocator(org='TestU', course='sig101', run='Summer2022', branch=None, version_guid=None),
+            name='Signals 101',
+            schedule_data=CourseScheduleData(
+                start=datetime.fromisoformat('2030-01-01T00:00+00:00'),
+                pacing='instructor',
+                end=None,
+                enrollment_start=None,
+                enrollment_end=None),
+            short_description=None,
+            effort=None,
+            hidden=False,
+            invitation_only=False
+        )
+
+    @patch(
+        'cms.djangoapps.contentstore.signals.handlers.transaction.on_commit',
+        autospec=True, side_effect=lambda func: func(),  # run right away
+    )
+    @patch('cms.djangoapps.contentstore.signals.handlers.emit_catalog_info_changed_signal', autospec=True)
+    def test_signal_chain(self, mock_emit, _mock_on_commit):
+        """
+        Test that the course_published signal handler invokes the catalog info signal emitter.
+
+        I tested this in a bit of a weird way because I couldn't get the transaction on-commit
+        to run during the test, so instead I capture it and call the callbacks right away.
+        """
+        with SignalHandler.course_published.for_state(is_enabled=True):
+            SignalHandler.course_published.send(TestCatalogInfoSignal, course_key=self.course_key)
+        mock_emit.assert_called_once_with(self.course_key)
+
+    @override_settings(SEND_CATALOG_INFO_SIGNAL=True)
+    @patch('cms.djangoapps.contentstore.signals.handlers.COURSE_CATALOG_INFO_CHANGED', autospec=True)
+    def test_emit_regular_course(self, mock_signal):
+        """On a normal course publish, send an event."""
+        sh.emit_catalog_info_changed_signal(self.course_key)
+        mock_signal.send_event.assert_called_once_with(catalog_info=self.expected_data)
+
+    @override_settings(SEND_CATALOG_INFO_SIGNAL=True)
+    @patch('cms.djangoapps.contentstore.signals.handlers.COURSE_CATALOG_INFO_CHANGED', autospec=True)
+    def test_ignore_library(self, mock_signal):
+        """When course key is actually a library, don't send."""
+        sh.emit_catalog_info_changed_signal(LibraryLocator(org='SomeOrg', library='stuff'))
+        mock_signal.send_event.assert_not_called()
+
+    @override_settings(SEND_CATALOG_INFO_SIGNAL=False)
+    @patch('cms.djangoapps.contentstore.signals.handlers.COURSE_CATALOG_INFO_CHANGED', autospec=True)
+    def test_disabled(self, mock_signal):
+        """When toggle is disabled, don't send."""
+        sh.emit_catalog_info_changed_signal(self.course_key)
+        mock_signal.send_event.assert_not_called()
diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py
@@ -295,6 +295,10 @@ def should_show_debug_toolbar(request):  # lint-amnesty, pylint: disable=missing
 CREDENTIALS_INTERNAL_SERVICE_URL = 'http://localhost:18150'
 CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:18150'
 
+#################### Event bus backend ########################
+EVENT_BUS_KAFKA_SCHEMA_REGISTRY_URL = 'http://edx.devstack.schema-registry:8081'
+EVENT_BUS_KAFKA_BOOTSTRAP_SERVERS = 'edx.devstack.kafka:29092'
+
 ################# New settings must go ABOVE this line #################
 ########################################################################
 # See if the developer has any local overrides.

diff --git a/requirements/edx/base.in b/requirements/edx/base.in
@@ -84,6 +84,7 @@ edx-django-sites-extensions
 edx-django-utils>=5.0.0            # Utilities for cache, monitoring, and plugins
 edx-drf-extensions
 edx-enterprise
+edx-event-bus-kafka>=0.4.1          # Kafka implementation of event bus
 edx-milestones
 edx-name-affirmation
 edx-opaque-keys
@@ -123,7 +124,7 @@ nltk                                # Natural language processing; used by the c
 nodeenv                             # Utility for managing Node.js environments; we use this for deployments and testing
 oauthlib                            # OAuth specification support for authenticating via LTI or other Open edX services
 openedx-calc                        # Library supporting mathematical calculations for Open edX
-openedx-events                      # Open edX Events from Hooks Extension Framework (OEP-50)
+openedx-events>=0.12.0              # Open edX Events from Hooks Extension Framework (OEP-50)
 openedx-filters                     # Open edX Filters from Hooks Extension Framework (OEP-50)
 optimizely-sdk                      # Optimizely full stack SDK for Python
 ora2>=4.4.0

diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
@@ -223,6 +223,7 @@ django==3.2.15
     #   edx-django-utils
     #   edx-drf-extensions
     #   edx-enterprise
+    #   edx-event-bus-kafka
     #   edx-i18n-tools
     #   edx-milestones
     #   edx-name-affirmation
@@ -459,6 +460,7 @@ edx-django-utils==5.0.0
     #   django-config-models
     #   edx-drf-extensions
     #   edx-enterprise
+    #   edx-event-bus-kafka
     #   edx-name-affirmation
     #   edx-rest-api-client
     #   edx-toggles
@@ -485,6 +487,8 @@ edx-enterprise==3.56.5
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.in
     #   learner-pathway-progress
+edx-event-bus-kafka==0.4.1
+    # via -r requirements/edx/base.in
 edx-i18n-tools==0.9.1
     # via ora2
 edx-milestones==0.4.0
@@ -539,6 +543,7 @@ edx-toggles==5.0.0
     # via
     #   -r requirements/edx/base.in
     #   edx-completion
+    #   edx-event-bus-kafka
     #   edx-name-affirmation
     #   edxval
     #   learner-pathway-progress
@@ -754,8 +759,10 @@ oauthlib==3.0.1
     #   social-auth-core
 openedx-calc==3.0.1
     # via -r requirements/edx/base.in
-openedx-events==0.11.1
-    # via -r requirements/edx/base.in
+openedx-events==0.12.0
+    # via
+    #   -r requirements/edx/base.in
+    #   edx-event-bus-kafka
 openedx-filters==0.8.0
     # via
     #   -r requirements/edx/base.in

diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt
@@ -312,6 +312,7 @@ django==3.2.15
     #   edx-django-utils
     #   edx-drf-extensions
     #   edx-enterprise
+    #   edx-event-bus-kafka
     #   edx-i18n-tools
     #   edx-milestones
     #   edx-name-affirmation
@@ -573,6 +574,7 @@ edx-django-utils==5.0.0
     #   django-config-models
     #   edx-drf-extensions
     #   edx-enterprise
+    #   edx-event-bus-kafka
     #   edx-name-affirmation
     #   edx-rest-api-client
     #   edx-toggles
@@ -599,6 +601,8 @@ edx-enterprise==3.56.5
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/testing.txt
     #   learner-pathway-progress
+edx-event-bus-kafka==0.4.1
+    # via -r requirements/edx/testing.txt
 edx-i18n-tools==0.9.1
     # via
     #   -r requirements/edx/testing.txt
@@ -662,6 +666,7 @@ edx-toggles==5.0.0
     # via
     #   -r requirements/edx/testing.txt
     #   edx-completion
+    #   edx-event-bus-kafka
     #   edx-name-affirmation
     #   edxval
     #   learner-pathway-progress
@@ -982,8 +987,10 @@ oauthlib==3.0.1
     #   social-auth-core
 openedx-calc==3.0.1
     # via -r requirements/edx/testing.txt
-openedx-events==0.11.1
-    # via -r requirements/edx/testing.txt
+openedx-events==0.12.0
+    # via
+    #   -r requirements/edx/testing.txt
+    #   edx-event-bus-kafka
 openedx-filters==0.8.0
     # via
     #   -r requirements/edx/testing.txt

diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt
@@ -296,6 +296,7 @@ distlib==0.3.5
     #   edx-django-utils
     #   edx-drf-extensions
     #   edx-enterprise
+    #   edx-event-bus-kafka
     #   edx-i18n-tools
     #   edx-milestones
     #   edx-name-affirmation
@@ -553,6 +554,7 @@ edx-django-utils==5.0.0
     #   django-config-models
     #   edx-drf-extensions
     #   edx-enterprise
+    #   edx-event-bus-kafka
     #   edx-name-affirmation
     #   edx-rest-api-client
     #   edx-toggles
@@ -579,6 +581,8 @@ edx-enterprise==3.56.5
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt
     #   learner-pathway-progress
+edx-event-bus-kafka==0.4.1
+    # via -r requirements/edx/base.txt
 edx-i18n-tools==0.9.1
     # via
     #   -r requirements/edx/base.txt
@@ -641,6 +645,7 @@ edx-toggles==5.0.0
     # via
     #   -r requirements/edx/base.txt
     #   edx-completion
+    #   edx-event-bus-kafka
     #   edx-name-affirmation
     #   edxval
     #   learner-pathway-progress
@@ -933,8 +938,10 @@ oauthlib==3.0.1
     #   social-auth-core
 openedx-calc==3.0.1
     # via -r requirements/edx/base.txt
-openedx-events==0.11.1
-    # via -r requirements/edx/base.txt
+openedx-events==0.12.0
+    # via
+    #   -r requirements/edx/base.txt
+    #   edx-event-bus-kafka
 openedx-filters==0.8.0
     # via
     #   -r requirements/edx/base.txt

diff --git a/xmodule/modulestore/django.py b/xmodule/modulestore/django.py
@@ -4,6 +4,7 @@
 Passes settings.MODULESTORE as kwargs to MongoModuleStore
 """
 
+from contextlib import contextmanager
 from importlib import import_module
 import gettext
 import logging
@@ -87,6 +88,18 @@ def enable(self):
         """
         self._allow_signals = True
 
+    @contextmanager
+    def for_state(self, *, is_enabled: bool):
+        """
+        Set signal handling to be on or off for the duration of the context.
+        """
+        try:
+            old_state = self._allow_signals
+            self._allow_signals = is_enabled
+            yield
+        finally:
+            self._allow_signals = old_state
+
     def send(self, *args, **kwargs):
         """
         See `django.dispatch.Signal.send()`