Merge pull request #78 from openedx/cag/ccx-support

fix: sink ccx courses on original course published

.coveragerc

@@ -3,7 +3,6 @@ branch = True
 data_file = .coverage
 source=event_sink_clickhouse
 omit =
-    test_settings
     *migrations*
     *admin.py
     *static*

docs/conf.py

@@ -41,7 +41,7 @@ def get_version(*file_paths):
 
 VERSION = get_version('../event_sink_clickhouse', '__init__.py')
 # Configure Django for autodoc usage
-os.environ['DJANGO_SETTINGS_MODULE'] = 'test_settings'
+os.environ['DJANGO_SETTINGS_MODULE'] = 'test_utils.test_settings'
 django_setup()
 
 # If extensions (or modules to document with autodoc) are in another directory,

event_sink_clickhouse/__init__.py

@@ -2,4 +2,4 @@
 A sink for Open edX events to send them to ClickHouse.
 """
 
-__version__ = "1.1.0"
+__version__ = "1.1.1"

event_sink_clickhouse/settings/common.py

@@ -40,4 +40,8 @@ def plugin_settings(settings):
             "module": "openedx.core.djangoapps.external_user_ids.models",
             "model": "ExternalId",
         },
+        "custom_course_edx": {
+            "module": "lms.djangoapps.ccx.models",
+            "model": "CustomCourseForEdX",
+        }
     }

event_sink_clickhouse/signals.py

@@ -5,6 +5,7 @@
 from django.dispatch import Signal, receiver
 
 from event_sink_clickhouse.sinks.external_id_sink import ExternalIdSink
+from event_sink_clickhouse.sinks.user_profile_sink import UserProfileSink
 from event_sink_clickhouse.sinks.user_retire import UserRetirementSink
 from event_sink_clickhouse.utils import get_model
 
@@ -35,9 +36,14 @@ def on_user_profile_updated(  # pylint: disable=unused-argument  # pragma: no co
     Receives post save signal and queues the dump job.
     """
     # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded
-    from event_sink_clickhouse.tasks import dump_user_profile_to_clickhouse  # pylint: disable=import-outside-toplevel
+    from event_sink_clickhouse.tasks import dump_data_to_clickhouse  # pylint: disable=import-outside-toplevel
 
-    dump_user_profile_to_clickhouse.delay(instance.id)
+    sink = UserProfileSink(None, None)
+    dump_data_to_clickhouse.delay(
+        sink_module=sink.__module__,
+        sink_name=sink.__class__.__name__,
+        object_id=str(instance.id),
+    )
 
 
 @receiver(post_save, sender=get_model("external_id"))

event_sink_clickhouse/tasks.py

@@ -10,7 +10,7 @@
 from opaque_keys.edx.keys import CourseKey
 
 from event_sink_clickhouse.sinks.course_published import CourseOverviewSink
-from event_sink_clickhouse.sinks.user_profile_sink import UserProfileSink
+from event_sink_clickhouse.utils import get_ccx_courses
 
 log = logging.getLogger(__name__)
 celery_log = logging.getLogger("edx.celery.task")
@@ -32,23 +32,10 @@ def dump_course_to_clickhouse(course_key_string, connection_overrides=None):
         sink = CourseOverviewSink(connection_overrides=connection_overrides, log=celery_log)
         sink.dump(course_key)
 
-
-@shared_task
-@set_code_owner_attribute
-def dump_user_profile_to_clickhouse(user_profile_id, connection_overrides=None):
-    """
-    Serialize a user profile and writes it to ClickHouse.
-
-    Arguments:
-        user_profile_id: user profile id for the user profile to be exported
-        connection_overrides (dict):  overrides to ClickHouse connection
-            parameters specified in `settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG`.
-    """
-    if UserProfileSink.is_enabled():  # pragma: no cover
-        sink = UserProfileSink(
-            connection_overrides=connection_overrides, log=celery_log
-        )
-        sink.dump(user_profile_id)
+        ccx_courses = get_ccx_courses(course_key)
+        for ccx_course in ccx_courses:
+            ccx_course_key = str(ccx_course.locator)
+            sink.dump(ccx_course_key)
 
 
 @shared_task

event_sink_clickhouse/utils.py

@@ -57,3 +57,12 @@ def get_detached_xblock_types():  # pragma: no cover
     from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES
 
     return DETACHED_XBLOCK_TYPES
+
+
+def get_ccx_courses(course_id):
+    """
+    Get the CCX courses for a given course.
+    """
+    if settings.FEATURES.get("CUSTOM_COURSES_EDX"):
+        return get_model("custom_course_edx").objects.filter(course_id=course_id)
+    return []

test_utils/test_settings.py

@@ -42,3 +42,7 @@
 }
 
 EVENT_SINK_CLICKHOUSE_COURSE_OVERVIEWS_ENABLED = True
+
+FEATURES = {
+    'CUSTOM_COURSES_EDX': True,
+}

test_utils/test_utils.py

@@ -6,7 +6,7 @@
 
 from django.conf import settings
 
-from event_sink_clickhouse.utils import get_model
+from event_sink_clickhouse.utils import get_ccx_courses, get_model
 
 
 class TestUtils(unittest.TestCase):
@@ -71,3 +71,17 @@ def test_get_model_missing_module_and_model_2(self):
     def test_get_model_missing_model_config(self):
         model = get_model("my_model")
         self.assertIsNone(model)
+
+    @patch("event_sink_clickhouse.utils.get_model")
+    def test_get_ccx_courses(self, mock_get_model):
+        mock_get_model.return_value = mock_model = Mock()
+
+        get_ccx_courses('dummy_key')
+
+        mock_model.objects.filter.assert_called_once_with(course_id='dummy_key')
+
+    @patch.object(settings, "FEATURES", {"CUSTOM_COURSES_EDX": False})
+    def test_get_ccx_courses_feature_disabled(self):
+        courses = get_ccx_courses('dummy_key')
+
+        self.assertEqual(list(courses), [])

tests/test_course_published.py

@@ -33,7 +33,14 @@
 @patch("event_sink_clickhouse.sinks.course_published.CourseOverviewSink.get_model")
 @patch("event_sink_clickhouse.sinks.course_published.get_detached_xblock_types")
 @patch("event_sink_clickhouse.sinks.course_published.get_modulestore")
-def test_course_publish_success(mock_modulestore, mock_detached, mock_overview, mock_serialize_item):
+@patch("event_sink_clickhouse.tasks.get_ccx_courses")
+def test_course_publish_success(
+    mock_get_ccx_courses,
+    mock_modulestore,
+    mock_detached,
+    mock_overview,
+    mock_serialize_item
+):
     """
     Test of a successful end-to-end run.
     """
@@ -48,6 +55,7 @@ def test_course_publish_success(mock_modulestore, mock_detached, mock_overview,
     mock_detached.return_value = mock_detached_xblock_types()
 
     mock_overview.return_value.get_from_id.return_value = course_overview
+    mock_get_ccx_courses.return_value = []
 
     # Use the responses library to catch the POSTs to ClickHouse
     # and match them against the expected values, including CSV
@@ -75,6 +83,7 @@ def test_course_publish_success(mock_modulestore, mock_detached, mock_overview,
     # Just to make sure we're not calling things more than we need to
     assert mock_modulestore.call_count == 1
     assert mock_detached.call_count == 1
+    mock_get_ccx_courses.assert_called_once_with(course_overview.id)
 
 
 @responses.activate(registry=OrderedRegistry)  # pylint: disable=unexpected-keyword-arg,no-value-for-parameter

tests/test_signals.py

@@ -5,12 +5,7 @@
 
 from django.test import TestCase
 
-from event_sink_clickhouse.signals import (
-    on_externalid_saved,
-    on_user_profile_updated,
-    on_user_retirement,
-    receive_course_publish,
-)
+from event_sink_clickhouse.signals import on_externalid_saved, on_user_retirement, receive_course_publish
 from event_sink_clickhouse.sinks.external_id_sink import ExternalIdSink
 from event_sink_clickhouse.sinks.user_retire import UserRetirementSink
 
@@ -31,17 +26,6 @@ def test_receive_course_publish(self, mock_dump_task):
 
         mock_dump_task.delay.assert_called_once_with(course_key)
 
-    @patch("event_sink_clickhouse.tasks.dump_user_profile_to_clickhouse")
-    def test_on_user_profile_updated(self, mock_dump_task):
-        """
-        Test that on_user_profile_updated calls dump_user_profile_to_clickhouse.
-        """
-        instance = Mock()
-        sender = Mock()
-        on_user_profile_updated(sender, instance)
-
-        mock_dump_task.delay.assert_called_once_with(instance.id)
-
     @patch("event_sink_clickhouse.tasks.dump_data_to_clickhouse")
     def test_on_externalid_saved(self, mock_dump_task):
         """

tests/test_tasks.py

@@ -4,36 +4,14 @@
 import unittest
 from unittest.mock import MagicMock, patch
 
-from event_sink_clickhouse.tasks import dump_data_to_clickhouse, dump_user_profile_to_clickhouse
+from event_sink_clickhouse.tasks import dump_data_to_clickhouse
 
 
 class TestTasks(unittest.TestCase):
     """
     Test cases for tasks.
     """
 
-    @patch("event_sink_clickhouse.tasks.UserProfileSink.is_enabled", return_value=True)
-    @patch("event_sink_clickhouse.tasks.UserProfileSink")
-    @patch("event_sink_clickhouse.tasks.celery_log")
-    def test_dump_user_profile_to_clickhouse(
-        self, mock_celery_log, mock_UserProfileSink, mock_is_enabled
-    ):
-        # Mock the required objects and methods
-        mock_sink_instance = mock_UserProfileSink.return_value
-        mock_sink_instance.dump.return_value = None
-
-        # Call the function
-        dump_user_profile_to_clickhouse(
-            "user_profile_id", connection_overrides={"param": "value"}
-        )
-
-        # Assertions
-        mock_is_enabled.assert_called_once()
-        mock_UserProfileSink.assert_called_once_with(
-            connection_overrides={"param": "value"}, log=mock_celery_log
-        )
-        mock_sink_instance.dump.assert_called_once_with("user_profile_id")
-
     @patch("event_sink_clickhouse.tasks.import_module")
     @patch("event_sink_clickhouse.tasks.celery_log")
     def test_dump_data_to_clickhouse(self, mock_celery_log, mock_import_module):

tox.ini

@@ -78,12 +78,12 @@ commands =
     rm tests/__init__.py
     pycodestyle event_sink_clickhouse tests manage.py setup.py
     pydocstyle event_sink_clickhouse tests manage.py setup.py
-    isort --check-only --diff tests test_utils event_sink_clickhouse manage.py setup.py test_settings.py
+    isort --check-only --diff tests test_utils event_sink_clickhouse manage.py setup.py
     make selfcheck
 
 [testenv:pii_check]
 setenv =
-    DJANGO_SETTINGS_MODULE = test_settings
+    DJANGO_SETTINGS_MODULE = test_utils.test_settings
 deps =
     -r{toxinidir}/requirements/test.txt
 commands =