From da2052ab34d7e4dec7d4cd8b10bbf2fd2d71cff7 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 12 Jun 2024 11:24:16 +0930 Subject: [PATCH 1/9] fix: catch ImportErrors so that event-routing-backends can be dependency for non-edx-platform repos like openedx-completion-aggregator. --- event_routing_backends/helpers.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/event_routing_backends/helpers.py b/event_routing_backends/helpers.py index 1b0fcdd0..26e4204f 100644 --- a/event_routing_backends/helpers.py +++ b/event_routing_backends/helpers.py @@ -6,20 +6,29 @@ import uuid from urllib.parse import parse_qs, urlparse -# Imported from edx-platform -# pylint: disable=import-error -from common.djangoapps.student.models import get_potentially_retired_user_by_username from dateutil.parser import parse from django.conf import settings from django.contrib.auth import get_user_model from isodate import duration_isoformat from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.content.course_overviews.api import get_course_overviews -from openedx.core.djangoapps.external_user_ids.models import ExternalId, ExternalIdType logger = logging.getLogger(__name__) -User = get_user_model() +# Imported from edx-platform +try: + from common.djangoapps.student.models import get_potentially_retired_user_by_username + from openedx.core.djangoapps.content.course_overviews.api import get_course_overviews + from openedx.core.djangoapps.external_user_ids.models import ExternalId, ExternalIdType +except ImportError as exc: # pragma: no cover + logger.exception(exc) + + get_potentially_retired_user_by_username = None + get_course_overviews = None + ExternalId = None + ExternalIdType = None + + +User = get_user_model() UTC_DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%S.%f' BLOCK_ID_FORMAT = '{block_version}:{course_id}+type@{block_type}+block@{block_id}' @@ -57,6 +66,9 @@ def get_anonymous_user_id(username_or_id, external_type): Returns: str """ + if not (ExternalId and ExternalIdType): + raise ImportError("Could not import external_user_ids from edx-platform.") # pragma: no cover + user = get_user(username_or_id) if not user: logger.warning('User with username "%s" does not exist. ' @@ -94,6 +106,9 @@ def get_user(username_or_id): Returns: user object """ + if not get_potentially_retired_user_by_username: + raise ImportError("Could not import student.models from edx-platform.") # pragma: no cover + user = username = None if not username_or_id: @@ -145,6 +160,9 @@ def get_course_from_id(course_id): Returns: Course """ + if not get_course_overviews: + raise ImportError("Could not course_overviews.api from edx-platform.") # pragma: no cover + course_key = CourseKey.from_string(course_id) course_overviews = get_course_overviews([course_key]) if course_overviews: From a415ee72083c20961625b14072a4061931686f11 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 12 Jun 2024 14:35:25 +0930 Subject: [PATCH 2/9] feat: adds utils.settings.event_tracking_backends_config() * Pulls the EVENT_TRACKING_BACKENDS configuration template into a utility method so it can be used by other apps. * Pulls allowed xAPI and Caliper events into settings to preserve changes * Adds a test --- event_routing_backends/settings/common.py | 98 ++++++------------- event_routing_backends/settings/production.py | 8 ++ event_routing_backends/tests/test_settings.py | 24 ++++- event_routing_backends/utils/settings.py | 82 ++++++++++++++++ 4 files changed, 140 insertions(+), 72 deletions(-) create mode 100644 event_routing_backends/utils/settings.py diff --git a/event_routing_backends/settings/common.py b/event_routing_backends/settings/common.py index 0bf98370..a6bfe124 100644 --- a/event_routing_backends/settings/common.py +++ b/event_routing_backends/settings/common.py @@ -2,6 +2,8 @@ Default settings for the event_routing_backends app. """ +from ..utils.settings import event_tracking_backends_config + def plugin_settings(settings): """ @@ -77,7 +79,15 @@ def plugin_settings(settings): 'edx.course.enrollment.deactivated', 'edx.course.grade.passed.first_time' ] - allowed_xapi_events = [ + # .. setting_name: EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS + # .. setting_default: [ + # 'edx.course.enrollment.activated', + # 'edx.course.enrollment.deactivated', + # 'edx.course.grade.passed.first_time', + # ... + # ] + # .. setting_description: Contains the full list of events to be processed by the xAPI backend. + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS = [ 'edx.course.enrollment.activated', 'edx.course.enrollment.deactivated', 'edx.course.enrollment.mode_changed', @@ -144,7 +154,15 @@ def plugin_settings(settings): 'edx.course.grade.now_passed', 'edx.course.grade.now_failed', ] - allowed_caliper_events = [ + # .. setting_name: EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS + # .. setting_default: [ + # 'edx.course.enrollment.activated', + # 'edx.course.enrollment.deactivated', + # 'edx.course.grade.passed.first_time', + # ... + # ] + # .. setting_description: Contains the full list of events to be processed by the Caliper backend. + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS = [ 'edx.course.enrollment.activated', 'edx.course.enrollment.deactivated', 'edx.ui.lms.link_clicked', @@ -173,75 +191,15 @@ def plugin_settings(settings): 'edx.course.grade.now_failed' ] - allowed_events = set(allowed_xapi_events + allowed_caliper_events) - # Operators can configure the event bus allowed events via EVENT_BUS_TRACKING_LOGS and by default # we are allowing the supported events by xAPI and Caliper so that operators don't need to configure # the events manually. - settings.EVENT_BUS_TRACKING_LOGS = allowed_events + settings.EVENT_BUS_TRACKING_LOGS = set( + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS + + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS + ) - settings.EVENT_TRACKING_BACKENDS.update({ - 'event_transformer': { - 'ENGINE': 'eventtracking.backends.async_routing.AsyncRoutingBackend', - 'OPTIONS': { - 'backend_name': 'event_transformer', - 'processors': [ - { - 'ENGINE': 'eventtracking.processors.whitelist.NameWhitelistProcessor', - 'OPTIONS': { - 'whitelist': allowed_events - } - }, - ], - 'backends': { - 'xapi': { - 'ENGINE': 'event_routing_backends.backends.async_events_router.AsyncEventsRouter', - 'OPTIONS': { - 'processors': [ - { - 'ENGINE': 'eventtracking.processors.whitelist.NameWhitelistProcessor', - 'OPTIONS': { - 'whitelist': allowed_xapi_events - } - }, - { - 'ENGINE': - 'event_routing_backends.processors.xapi.transformer_processor.XApiProcessor', - 'OPTIONS': {} - } - ], - 'backend_name': 'xapi', - } - }, - "caliper": { - 'ENGINE': 'event_routing_backends.backends.async_events_router.AsyncEventsRouter', - "OPTIONS": { - "processors": [ - { - "ENGINE": "eventtracking.processors.whitelist.NameWhitelistProcessor", - "OPTIONS": { - "whitelist": allowed_caliper_events - } - }, - { - "ENGINE": - "event_routing_backends.processors." - "caliper.transformer_processor.CaliperProcessor", - "OPTIONS": {} - }, - { - "ENGINE": - "event_routing_backends.processors." - "caliper.envelope_processor.CaliperEnvelopeProcessor", - "OPTIONS": { - "sensor_id": settings.LMS_ROOT_URL - } - } - ], - "backend_name": "caliper" - } - } - }, - }, - }, - }) + settings.EVENT_TRACKING_BACKENDS.update(event_tracking_backends_config( + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS, + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS, + )) diff --git a/event_routing_backends/settings/production.py b/event_routing_backends/settings/production.py index 646bfc02..c530e7e9 100644 --- a/event_routing_backends/settings/production.py +++ b/event_routing_backends/settings/production.py @@ -51,6 +51,14 @@ def plugin_settings(settings): 'EVENT_TRACKING_BACKENDS_BUSINESS_CRITICAL_EVENTS', settings.EVENT_TRACKING_BACKENDS_BUSINESS_CRITICAL_EVENTS ) + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS = settings.ENV_TOKENS.get( + 'EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS', + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS + ) + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS = settings.ENV_TOKENS.get( + 'EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS', + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS + ) settings.XAPI_AGENT_IFI_TYPE = settings.ENV_TOKENS.get( 'XAPI_AGENT_IFI_TYPE', settings.XAPI_AGENT_IFI_TYPE diff --git a/event_routing_backends/tests/test_settings.py b/event_routing_backends/tests/test_settings.py index 13514991..3abe5357 100644 --- a/event_routing_backends/tests/test_settings.py +++ b/event_routing_backends/tests/test_settings.py @@ -20,9 +20,29 @@ def test_common_settings(self): Test common settings """ common_settings.plugin_settings(settings) + self.assertIn('event_transformer', settings.EVENT_TRACKING_BACKENDS) - self.assertIn('xapi', settings.EVENT_TRACKING_BACKENDS["event_transformer"]["OPTIONS"]["backends"]) - self.assertIn('caliper', settings.EVENT_TRACKING_BACKENDS["event_transformer"]["OPTIONS"]["backends"]) + self.assertIn('OPTIONS', settings.EVENT_TRACKING_BACKENDS["event_transformer"]) + transformer_options = settings.EVENT_TRACKING_BACKENDS["event_transformer"]["OPTIONS"] + + self.assertEqual( + set( + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS + + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS, + ), + transformer_options["processors"][0]["OPTIONS"]["whitelist"] + ) + + self.assertIn("xapi", transformer_options["backends"]) + self.assertEqual( + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS, + transformer_options["backends"]["xapi"]["OPTIONS"]["processors"][0]["OPTIONS"]["whitelist"]) + + self.assertIn("caliper", settings.EVENT_TRACKING_BACKENDS["event_transformer"]["OPTIONS"]["backends"]) + self.assertEqual( + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS, + transformer_options["backends"]["caliper"]["OPTIONS"]["processors"][0]["OPTIONS"]["whitelist"]) + self.assertIn('edx.course.enrollment.activated', settings.EVENT_TRACKING_BACKENDS_BUSINESS_CRITICAL_EVENTS) self.assertFalse(settings.CALIPER_EVENTS_ENABLED) self.assertFalse(settings.CALIPER_EVENT_LOGGING_ENABLED) diff --git a/event_routing_backends/utils/settings.py b/event_routing_backends/utils/settings.py new file mode 100644 index 00000000..aded5930 --- /dev/null +++ b/event_routing_backends/utils/settings.py @@ -0,0 +1,82 @@ +""" +Helper utilities for event transformer settings. +""" + +from typing import List + +from django.conf import settings + + +def event_tracking_backends_config(allowed_xapi_events: List[str], allowed_caliper_events: List[str]) -> dict: + """ + Return the recommended settings.EVENT_TRACKING_BACKENDS configuration. + + Include the given `allowed_xapi_events` and `allowed_caliper_events` in the relevant backend whitelists. + """ + all_allowed_events = set(allowed_xapi_events + allowed_caliper_events) + + return { + 'event_transformer': { + 'ENGINE': 'eventtracking.backends.async_routing.AsyncRoutingBackend', + 'OPTIONS': { + 'backend_name': 'event_transformer', + 'processors': [ + { + 'ENGINE': 'eventtracking.processors.whitelist.NameWhitelistProcessor', + 'OPTIONS': { + 'whitelist': all_allowed_events + } + }, + ], + 'backends': { + 'xapi': { + 'ENGINE': 'event_routing_backends.backends.async_events_router.AsyncEventsRouter', + 'OPTIONS': { + 'processors': [ + { + 'ENGINE': 'eventtracking.processors.whitelist.NameWhitelistProcessor', + 'OPTIONS': { + 'whitelist': allowed_xapi_events + } + }, + { + 'ENGINE': + 'event_routing_backends.processors.xapi.transformer_processor.XApiProcessor', + 'OPTIONS': {} + } + ], + 'backend_name': 'xapi', + } + }, + "caliper": { + 'ENGINE': 'event_routing_backends.backends.async_events_router.AsyncEventsRouter', + "OPTIONS": { + "processors": [ + { + "ENGINE": "eventtracking.processors.whitelist.NameWhitelistProcessor", + "OPTIONS": { + "whitelist": allowed_caliper_events + } + }, + { + "ENGINE": + "event_routing_backends.processors." + "caliper.transformer_processor.CaliperProcessor", + "OPTIONS": {} + }, + { + "ENGINE": + "event_routing_backends.processors." + "caliper.envelope_processor.CaliperEnvelopeProcessor", + "OPTIONS": { + "sensor_id": settings.LMS_ROOT_URL + } + } + ], + "backend_name": "caliper" + } + } + }, + }, + } + } From 47c25fb2f8d829e99806510589be804bb2d5aace Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 13 Jun 2024 15:48:36 +0930 Subject: [PATCH 3/9] fix: event_tracking_backends_config must use the provided plugin settings instead of global django.conf.settings --- event_routing_backends/settings/common.py | 1 + event_routing_backends/utils/settings.py | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/event_routing_backends/settings/common.py b/event_routing_backends/settings/common.py index a6bfe124..414cae67 100644 --- a/event_routing_backends/settings/common.py +++ b/event_routing_backends/settings/common.py @@ -200,6 +200,7 @@ def plugin_settings(settings): ) settings.EVENT_TRACKING_BACKENDS.update(event_tracking_backends_config( + settings, settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS, settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS, )) diff --git a/event_routing_backends/utils/settings.py b/event_routing_backends/utils/settings.py index aded5930..6161c6ca 100644 --- a/event_routing_backends/utils/settings.py +++ b/event_routing_backends/utils/settings.py @@ -4,10 +4,8 @@ from typing import List -from django.conf import settings - -def event_tracking_backends_config(allowed_xapi_events: List[str], allowed_caliper_events: List[str]) -> dict: +def event_tracking_backends_config(settings, allowed_xapi_events: List[str], allowed_caliper_events: List[str]) -> dict: """ Return the recommended settings.EVENT_TRACKING_BACKENDS configuration. From 5d42bf740e013f137cb53a0f5767439fddb5f60c Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 13 Jun 2024 19:02:52 +0930 Subject: [PATCH 4/9] fix: typos in class variable name and in a comment. --- event_routing_backends/helpers.py | 2 +- .../processors/caliper/tests/test_transformers.py | 2 +- .../processors/tests/transformers_test_mixin.py | 4 ++-- .../processors/xapi/tests/test_transformers.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/event_routing_backends/helpers.py b/event_routing_backends/helpers.py index 26e4204f..281640fa 100644 --- a/event_routing_backends/helpers.py +++ b/event_routing_backends/helpers.py @@ -161,7 +161,7 @@ def get_course_from_id(course_id): Course """ if not get_course_overviews: - raise ImportError("Could not course_overviews.api from edx-platform.") # pragma: no cover + raise ImportError("Could not import course_overviews.api from edx-platform.") # pragma: no cover course_key = CourseKey.from_string(course_id) course_overviews = get_course_overviews([course_key]) diff --git a/event_routing_backends/processors/caliper/tests/test_transformers.py b/event_routing_backends/processors/caliper/tests/test_transformers.py index bcefadda..088d8cf4 100644 --- a/event_routing_backends/processors/caliper/tests/test_transformers.py +++ b/event_routing_backends/processors/caliper/tests/test_transformers.py @@ -13,7 +13,7 @@ class TestCaliperTransformers(TransformersTestMixin, TestCase): """ Test that supported events are transformed into Caliper format correctly. """ - EXCEPTED_EVENTS_FIXTURES_PATH = '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) + EXPECTED_EVENTS_FIXTURES_PATH = '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) registry = CaliperTransformersRegistry diff --git a/event_routing_backends/processors/tests/transformers_test_mixin.py b/event_routing_backends/processors/tests/transformers_test_mixin.py index 0ca28e4e..a9c767b7 100644 --- a/event_routing_backends/processors/tests/transformers_test_mixin.py +++ b/event_routing_backends/processors/tests/transformers_test_mixin.py @@ -40,7 +40,7 @@ class TransformersTestMixin: maxDiff = None registry = None - EXCEPTED_EVENTS_FIXTURES_PATH = None + EXPECTED_EVENTS_FIXTURES_PATH = None def setUp(self): UserFactory.create(username='edx', email='edx@example.com') @@ -101,7 +101,7 @@ def test_event_transformer(self, event_filename, mocked_uuid4): # if an event's expected fixture doesn't exist, the test shouldn't fail. # evaluate transformation of only supported event fixtures. expected_event_file_path = '{expected_events_fixtures_path}/{event_filename}'.format( - expected_events_fixtures_path=self.EXCEPTED_EVENTS_FIXTURES_PATH, event_filename=event_filename + expected_events_fixtures_path=self.EXPECTED_EVENTS_FIXTURES_PATH, event_filename=event_filename ) if not os.path.isfile(expected_event_file_path): diff --git a/event_routing_backends/processors/xapi/tests/test_transformers.py b/event_routing_backends/processors/xapi/tests/test_transformers.py index 12a34649..5ed6e4fc 100644 --- a/event_routing_backends/processors/xapi/tests/test_transformers.py +++ b/event_routing_backends/processors/xapi/tests/test_transformers.py @@ -18,7 +18,7 @@ class TestXApiTransformers(TransformersTestMixin, TestCase): """ Test that supported events are transformed into xAPI format correctly. """ - EXCEPTED_EVENTS_FIXTURES_PATH = '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) + EXPECTED_EVENTS_FIXTURES_PATH = '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) registry = XApiTransformersRegistry def assert_correct_transformer_version(self, transformed_event, transformer_version): From 6e383b9b3ee2d54e9806c37f0503ffa3c0d1f7fb Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 13 Jun 2024 21:12:48 +0930 Subject: [PATCH 5/9] refactor: creates fixture test mixins that can be used outside of ERB Refactors the transformer test classes to split the test functionality into two: * TransformersFixturesTestMixin -- for running the event fixture tests * TransformersTestMixin -- for running the ERB-specific event tests The fixtures file path constants have been moved to property methods so they can be overrideen when used outside of ERB. --- .../caliper/tests/test_transformers.py | 26 +++- .../tests/transformers_test_mixin.py | 137 +++++++++++------- .../xapi/tests/test_transformers.py | 25 +++- 3 files changed, 128 insertions(+), 60 deletions(-) diff --git a/event_routing_backends/processors/caliper/tests/test_transformers.py b/event_routing_backends/processors/caliper/tests/test_transformers.py index 088d8cf4..3d3e3fe3 100644 --- a/event_routing_backends/processors/caliper/tests/test_transformers.py +++ b/event_routing_backends/processors/caliper/tests/test_transformers.py @@ -6,17 +6,27 @@ from django.test import TestCase from event_routing_backends.processors.caliper.registry import CaliperTransformersRegistry -from event_routing_backends.processors.tests.transformers_test_mixin import TransformersTestMixin +from event_routing_backends.processors.tests.transformers_test_mixin import ( + TransformersFixturesTestMixin, + TransformersTestMixin, +) -class TestCaliperTransformers(TransformersTestMixin, TestCase): +class CaliperTransformersFixturesTestMixin(TransformersFixturesTestMixin): """ - Test that supported events are transformed into Caliper format correctly. - """ - EXPECTED_EVENTS_FIXTURES_PATH = '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) + Mixin for testing Caliper event transformers. + This mixin is split into its own class so it can be used by packages outside of ERB. + """ registry = CaliperTransformersRegistry + @property + def expected_events_fixture_path(self): + """ + Return the path to the expected transformed events fixture files. + """ + return '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) + def assert_correct_transformer_version(self, transformed_event, transformer_version): self.assertEqual(transformed_event['extensions']['transformerVersion'], transformer_version) @@ -36,3 +46,9 @@ def compare_events(self, transformed_event, expected_event): expected_event.pop('id') transformed_event.pop('id') self.assertDictEqual(expected_event, transformed_event) + + +class TestCaliperTransformers(CaliperTransformersFixturesTestMixin, TransformersTestMixin, TestCase): + """ + Test that supported events are transformed into Caliper format correctly. + """ diff --git a/event_routing_backends/processors/tests/transformers_test_mixin.py b/event_routing_backends/processors/tests/transformers_test_mixin.py index a9c767b7..fec41d39 100644 --- a/event_routing_backends/processors/tests/transformers_test_mixin.py +++ b/event_routing_backends/processors/tests/transformers_test_mixin.py @@ -31,86 +31,67 @@ class DummyTransformer(BaseTransformerMixin): required_fields = ('does_not_exist',) -@ddt.ddt -class TransformersTestMixin: +class TransformersFixturesTestMixin: """ - Test that supported events are transformed correctly. + Mixin to help test event transforms using "raw" and "expected" fixture data. """ # no limit to diff in the output of tests maxDiff = None registry = None - EXPECTED_EVENTS_FIXTURES_PATH = None def setUp(self): + super().setUp() UserFactory.create(username='edx', email='edx@example.com') + @property + def raw_events_fixture_path(self): + """ + Return the path to the raw events fixture files. + """ + return f"{TEST_DIR_PATH}/fixtures/current" + + @property + def expected_events_fixture_path(self): + """ + Return the path to the expected transformed events fixture files. + """ + raise NotImplementedError + def get_raw_event(self, event_filename): """ Return raw event json parsed from current fixtures """ + base_event_filename = os.path.basename(event_filename) - input_event_file_path = '{test_dir}/fixtures/current/{event_filename}'.format( - test_dir=TEST_DIR_PATH, event_filename=event_filename + input_event_file_path = '{test_dir}/{event_filename}'.format( + test_dir=self.raw_events_fixture_path, event_filename=base_event_filename ) with open(input_event_file_path, encoding='utf-8') as current: data = json.loads(current.read()) return data - @override_settings(RUNNING_WITH_TEST_SETTINGS=True) - def test_transformer_version_with_test_settings(self): - self.registry.register('test_event')(DummyTransformer) - raw_event = self.get_raw_event('edx.course.enrollment.activated.json') - transformed_event = self.registry.get_transformer(raw_event).transform() - self.assert_correct_transformer_version(transformed_event, 'event-routing-backends@1.1.1') - - @override_settings(RUNNING_WITH_TEST_SETTINGS=False) - def test_transformer_version(self): - self.registry.register('test_event')(DummyTransformer) - raw_event = self.get_raw_event('edx.course.enrollment.activated.json') - transformed_event = self.registry.get_transformer(raw_event).transform() - self.assert_correct_transformer_version(transformed_event, 'event-routing-backends@{}'.format(__version__)) - - def test_with_no_field_transformer(self): - self.registry.register('test_event')(DummyTransformer) - with self.assertRaises(ValueError): - self.registry.get_transformer({ - 'name': 'test_event' - }).transform() - - def test_required_field_transformer(self): - self.registry.register('test_event')(DummyTransformer) - with self.assertRaises(ValueError): - self.registry.get_transformer({ - "name": "edx.course.enrollment.activated" - }).transform() - @abstractmethod def compare_events(self, transformed_event, expected_event): """ Every transformer's test case will implement its own logic to test events transformation """ - @patch('event_routing_backends.helpers.uuid.uuid4') - @ddt.data(*EVENT_FIXTURE_FILENAMES) - def test_event_transformer(self, event_filename, mocked_uuid4): - # Used to generate the anonymized actor.name, - # which in turn is used to generate the event UUID. - mocked_uuid4.return_value = UUID('32e08e30-f8ae-4ce2-94a8-c2bfe38a70cb') + raise NotImplementedError - # if an event's expected fixture doesn't exist, the test shouldn't fail. - # evaluate transformation of only supported event fixtures. - expected_event_file_path = '{expected_events_fixtures_path}/{event_filename}'.format( - expected_events_fixtures_path=self.EXPECTED_EVENTS_FIXTURES_PATH, event_filename=event_filename - ) + def check_event_transformer(self, raw_event_file, expected_event_file): + """ + Test that given event is transformed correctly. - if not os.path.isfile(expected_event_file_path): - return + Transforms the contents of `raw_event_file` and compare it against the contents of `expected_event_file`. - original_event = self.get_raw_event(event_filename) - with open(expected_event_file_path, encoding='utf-8') as expected: + Writes errors to test_out/ for analysis. + """ + original_event = self.get_raw_event(raw_event_file) + with open(expected_event_file, encoding='utf-8') as expected: expected_event = json.loads(expected.read()) + event_filename = os.path.basename(raw_event_file) if "anonymous" in event_filename: with pytest.raises(ValueError): self.registry.get_transformer(original_event).transform() @@ -132,7 +113,61 @@ def test_event_transformer(self, event_filename, mocked_uuid4): actual_transformed_event_file.write(",".join(out_events)) actual_transformed_event_file.write("]") - with open(f"test_output/expected.{event_filename}.json", "w") as expected_event_file: - json.dump(expected_event, expected_event_file, indent=4) + with open(f"test_output/expected.{raw_event_file}.json", "w") as test_output_file: + json.dump(expected_event, test_output_file, indent=4) raise e + + +@ddt.ddt +class TransformersTestMixin: + """ + Tests that supported events are transformed correctly. + """ + def test_with_no_field_transformer(self): + self.registry.register('test_event')(DummyTransformer) + with self.assertRaises(ValueError): + self.registry.get_transformer({ + 'name': 'test_event' + }).transform() + + def test_required_field_transformer(self): + self.registry.register('test_event')(DummyTransformer) + with self.assertRaises(ValueError): + self.registry.get_transformer({ + "name": "edx.course.enrollment.activated" + }).transform() + + @override_settings(RUNNING_WITH_TEST_SETTINGS=True) + def test_transformer_version_with_test_settings(self): + self.registry.register('test_event')(DummyTransformer) + raw_event = self.get_raw_event('edx.course.enrollment.activated.json') + transformed_event = self.registry.get_transformer(raw_event).transform() + self.assert_correct_transformer_version(transformed_event, 'event-routing-backends@1.1.1') + + @override_settings(RUNNING_WITH_TEST_SETTINGS=False) + def test_transformer_version(self): + self.registry.register('test_event')(DummyTransformer) + raw_event = self.get_raw_event('edx.course.enrollment.activated.json') + transformed_event = self.registry.get_transformer(raw_event).transform() + self.assert_correct_transformer_version(transformed_event, 'event-routing-backends@{}'.format(__version__)) + + @patch('event_routing_backends.helpers.uuid.uuid4') + @ddt.data(*EVENT_FIXTURE_FILENAMES) + def test_event_transformer(self, raw_event_file_path, mocked_uuid4): + # Used to generate the anonymized actor.name, + # which in turn is used to generate the event UUID. + mocked_uuid4.return_value = UUID('32e08e30-f8ae-4ce2-94a8-c2bfe38a70cb') + + # if an event's expected fixture doesn't exist, the test shouldn't fail. + # evaluate transformation of only supported event fixtures. + base_event_filename = os.path.basename(raw_event_file_path) + + expected_event_file_path = '{expected_events_fixture_path}/{event_filename}'.format( + expected_events_fixture_path=self.expected_events_fixture_path, event_filename=base_event_filename + ) + + if not os.path.isfile(expected_event_file_path): + return + + self.check_event_transformer(raw_event_file_path, expected_event_file_path) diff --git a/event_routing_backends/processors/xapi/tests/test_transformers.py b/event_routing_backends/processors/xapi/tests/test_transformers.py index 5ed6e4fc..29e260fe 100644 --- a/event_routing_backends/processors/xapi/tests/test_transformers.py +++ b/event_routing_backends/processors/xapi/tests/test_transformers.py @@ -8,19 +8,30 @@ from django.test import TestCase from django.test.utils import override_settings -from event_routing_backends.processors.tests.transformers_test_mixin import TransformersTestMixin +from event_routing_backends.processors.tests.transformers_test_mixin import ( + TransformersFixturesTestMixin, + TransformersTestMixin, +) from event_routing_backends.processors.xapi import constants from event_routing_backends.processors.xapi.registry import XApiTransformersRegistry from event_routing_backends.processors.xapi.transformer import XApiTransformer -class TestXApiTransformers(TransformersTestMixin, TestCase): +class XApiTransformersFixturesTestMixin(TransformersFixturesTestMixin): """ - Test that supported events are transformed into xAPI format correctly. + Mixin for testing xAPI event transformers. + + This mixin is split into its own class so it can be used by packages outside of ERB. """ - EXPECTED_EVENTS_FIXTURES_PATH = '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) registry = XApiTransformersRegistry + @property + def expected_events_fixture_path(self): + """ + Return the path to the expected transformed events fixture files. + """ + return '{}/fixtures/expected'.format(os.path.dirname(os.path.abspath(__file__))) + def assert_correct_transformer_version(self, transformed_event, transformer_version): self.assertEqual( transformed_event.context.extensions[constants.XAPI_TRANSFORMER_VERSION_KEY], @@ -68,6 +79,12 @@ def _compare_events(self, transformed_event, expected_event): transformed_event_json = json.loads(transformed_event.to_json()) self.assertDictEqual(expected_event, transformed_event_json) + +class TestXApiTransformers(XApiTransformersFixturesTestMixin, TransformersTestMixin, TestCase): + """ + Test xApi event transforms and settings. + """ + @override_settings(XAPI_AGENT_IFI_TYPE='mbox') def test_xapi_agent_ifi_settings_mbox(self): self.registry.register('test_event')(XApiTransformer) From f35c58a7df9110922a28dc48ad537d7ae7fb9dc1 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 13 Jun 2024 21:47:58 +0930 Subject: [PATCH 6/9] fix: catch FileNotFoundError in test mixin This exception may occur when using ERB transformer test mixins because the fixture test files are not installed with this package. --- .../tests/transformers_test_mixin.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/event_routing_backends/processors/tests/transformers_test_mixin.py b/event_routing_backends/processors/tests/transformers_test_mixin.py index fec41d39..07f79887 100644 --- a/event_routing_backends/processors/tests/transformers_test_mixin.py +++ b/event_routing_backends/processors/tests/transformers_test_mixin.py @@ -2,6 +2,7 @@ Mixin for testing transformers for all of the currently supported events """ import json +import logging import os from abc import abstractmethod from unittest.mock import patch @@ -16,15 +17,22 @@ from event_routing_backends.processors.mixins.base_transformer import BaseTransformerMixin from event_routing_backends.tests.factories import UserFactory +logger = logging.getLogger(__name__) User = get_user_model() TEST_DIR_PATH = os.path.dirname(os.path.abspath(__file__)) -EVENT_FIXTURE_FILENAMES = [ - event_file_name for event_file_name in os.listdir( - f'{TEST_DIR_PATH}/fixtures/current/' - ) if event_file_name.endswith(".json") -] +try: + EVENT_FIXTURE_FILENAMES = [ + event_file_name for event_file_name in os.listdir( + f'{TEST_DIR_PATH}/fixtures/current/' + ) if event_file_name.endswith(".json") + ] + +except FileNotFoundError as exc: # pragma: no cover + # This exception may happen when these test mixins are used outside of the ERB package. + logger.exception(exc) + EVENT_FIXTURE_FILENAMES = [] class DummyTransformer(BaseTransformerMixin): From c5a89fa26917a1b8394b02db178058088a61e1cf Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 20 Jun 2024 15:38:38 +0930 Subject: [PATCH 7/9] fix: ensure plugin loading order doesn't matter for event routing settings Initialize EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS and EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS only if they aren't already initialized, and append our events to them. This allows other plugins to modify these settings too. --- event_routing_backends/settings/common.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/event_routing_backends/settings/common.py b/event_routing_backends/settings/common.py index 414cae67..24a5f83e 100644 --- a/event_routing_backends/settings/common.py +++ b/event_routing_backends/settings/common.py @@ -87,7 +87,10 @@ def plugin_settings(settings): # ... # ] # .. setting_description: Contains the full list of events to be processed by the xAPI backend. - settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS = [ + # If this setting has already been initialized, we append to the existing list. + if not hasattr(settings, 'EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS'): + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS = [] + settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS += [ 'edx.course.enrollment.activated', 'edx.course.enrollment.deactivated', 'edx.course.enrollment.mode_changed', @@ -162,7 +165,10 @@ def plugin_settings(settings): # ... # ] # .. setting_description: Contains the full list of events to be processed by the Caliper backend. - settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS = [ + # If this setting has already been initialized, we append to the existing list. + if not hasattr(settings, 'EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS'): + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS = [] + settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS += [ 'edx.course.enrollment.activated', 'edx.course.enrollment.deactivated', 'edx.ui.lms.link_clicked', From dc3a0f2aad08d201c717796b4f64b435d6b0bf87 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 20 Jun 2024 15:57:05 +0930 Subject: [PATCH 8/9] test: test the "append to events settings" functionality and fix coverage --- event_routing_backends/settings/common.py | 2 ++ event_routing_backends/tests/test_settings.py | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/event_routing_backends/settings/common.py b/event_routing_backends/settings/common.py index 24a5f83e..dbea6c90 100644 --- a/event_routing_backends/settings/common.py +++ b/event_routing_backends/settings/common.py @@ -205,6 +205,8 @@ def plugin_settings(settings): settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS ) + if not hasattr(settings, 'EVENT_TRACKING_BACKENDS') or not settings.EVENT_TRACKING_BACKENDS: + settings.EVENT_TRACKING_BACKENDS = {} settings.EVENT_TRACKING_BACKENDS.update(event_tracking_backends_config( settings, settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS, diff --git a/event_routing_backends/tests/test_settings.py b/event_routing_backends/tests/test_settings.py index 3abe5357..f6957738 100644 --- a/event_routing_backends/tests/test_settings.py +++ b/event_routing_backends/tests/test_settings.py @@ -3,7 +3,7 @@ """ from django.conf import settings -from django.test import TestCase +from django.test import TestCase, override_settings from event_routing_backends.settings import common as common_settings from event_routing_backends.settings import devstack as devstack_settings @@ -82,3 +82,16 @@ def test_production_settings(self): self.assertTrue(settings.CALIPER_EVENT_LOGGING_ENABLED) self.assertFalse(settings.XAPI_EVENTS_ENABLED) self.assertTrue(settings.XAPI_EVENT_LOGGING_ENABLED) + + @override_settings( + EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS=["my.event.1"], + EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS=["my.event.2"], + ) + def test_settings_append_events(self): + common_settings.plugin_settings(settings) + + self.assertGreater(len(settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS), 1) + self.assertIn("my.event.1", settings.EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS) + + self.assertGreater(len(settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS), 1) + self.assertIn("my.event.2", settings.EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS) From 7ebd04143de3fd0aec8ae8c4115cc2d315c7aa9c Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 20 Jun 2024 16:03:48 +0930 Subject: [PATCH 9/9] chore: bumps version to 9.3.0 --- CHANGELOG.rst | 6 ++++-- event_routing_backends/__init__.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 97199372..6396377c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,8 +11,10 @@ Change Log .. There should always be an "Unreleased" section for changes pending release. -Unreleased -~~~~~~~~~~ +[9.3.0] + +* Support use of ERB for transforming non-openedx events + [9.2.1] * Add support for either 'whitelist' or 'registry.mapping' options (whitelist introduced in v9.0.0) diff --git a/event_routing_backends/__init__.py b/event_routing_backends/__init__.py index dbd65bc9..41d7f8df 100644 --- a/event_routing_backends/__init__.py +++ b/event_routing_backends/__init__.py @@ -2,4 +2,4 @@ Various backends for receiving edX LMS events.. """ -__version__ = '9.2.1' +__version__ = '9.3.0'