Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

fix: sink ccx courses on original course published #78

Merged
merged 4 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ branch = True
data_file = .coverage
source=event_sink_clickhouse
omit =
test_settings
*migrations*
*admin.py
*static*
Expand Down
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion event_sink_clickhouse/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
A sink for Open edX events to send them to ClickHouse.
"""

__version__ = "1.1.0"
__version__ = "1.1.1"
4 changes: 4 additions & 0 deletions event_sink_clickhouse/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
}
10 changes: 8 additions & 2 deletions event_sink_clickhouse/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"))
Expand Down
23 changes: 5 additions & 18 deletions event_sink_clickhouse/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions event_sink_clickhouse/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
61 changes: 0 additions & 61 deletions test_settings.py

This file was deleted.

4 changes: 4 additions & 0 deletions test_utils/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,7 @@
}

EVENT_SINK_CLICKHOUSE_COURSE_OVERVIEWS_ENABLED = True

FEATURES = {
'CUSTOM_COURSES_EDX': True,
}
16 changes: 15 additions & 1 deletion test_utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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), [])
11 changes: 10 additions & 1 deletion tests/test_course_published.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 1 addition & 17 deletions tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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):
"""
Expand Down
24 changes: 1 addition & 23 deletions tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Loading