From a8598fa1fac5e26ac212aa588e8527e727581742 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Fri, 3 Feb 2023 08:56:16 -0500 Subject: [PATCH] feat!: use mongo timestamp when sending course_catalog_info_changed events to the event bus (#31698) * feat!: update timestamp of catalog-info-changed signal When producing the catalog-info-changed signal, we now get the timestamp of the change from modulestore, rather than relying on the current time at the time of the event send. BREAKING CHANGE: This will adjust the meaning of the time metadata for new catalog-info-changed events. --------- Co-authored-by: Robert Raposa --- cms/djangoapps/contentstore/signals/handlers.py | 17 ++++++++++------- .../contentstore/signals/tests/test_handlers.py | 11 ++++++++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 44c8c8a126dd..8365f435c3fc 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -2,7 +2,7 @@ import logging -from datetime import datetime +from datetime import datetime, timezone from functools import wraps from typing import Optional @@ -66,7 +66,7 @@ def wrapper(*args, **kwargs): 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]: +def _create_catalog_data_for_signal(course_key: CourseKey) -> (Optional[datetime], Optional[CourseCatalogData]): """ Creates data for catalog-info-changed signal when course is published. @@ -74,16 +74,19 @@ def create_catalog_data_for_signal(course_key: CourseKey) -> Optional[CourseCata 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. + (datetime, CourseCatalogData): Tuple including the timestamp of the + event, and data for signal, or (None, None) if not appropriate + to send on this signal. """ # Only operate on real courses, not libraries. if not course_key.is_course: - return None + return None, None store = modulestore() with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key): course = store.get_course(course_key) - return CourseCatalogData( + timestamp = course.subtree_edited_on.replace(tzinfo=timezone.utc) + return timestamp, CourseCatalogData( course_key=course_key.for_branch(None), # Shouldn't be necessary, but just in case... name=course.display_name, schedule_data=CourseScheduleData( @@ -103,9 +106,9 @@ 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) + timestamp, 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) + COURSE_CATALOG_INFO_CHANGED.send_event(time=timestamp, catalog_info=catalog_info) @receiver(SignalHandler.course_published) diff --git a/cms/djangoapps/contentstore/signals/tests/test_handlers.py b/cms/djangoapps/contentstore/signals/tests/test_handlers.py index 8cb4c60018c5..2477e4c5b86f 100644 --- a/cms/djangoapps/contentstore/signals/tests/test_handlers.py +++ b/cms/djangoapps/contentstore/signals/tests/test_handlers.py @@ -2,7 +2,7 @@ Tests for signal handlers in the contentstore. """ -from datetime import datetime +from datetime import datetime, timezone from unittest.mock import patch from django.test.utils import override_settings @@ -10,6 +10,7 @@ from openedx_events.content_authoring.data import CourseCatalogData, CourseScheduleData import cms.djangoapps.contentstore.signals.handlers as sh +from xmodule.modulestore.edit_info import EditInfoMixin from xmodule.modulestore.django import SignalHandler from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import SampleCourseFactory @@ -63,8 +64,12 @@ def test_signal_chain(self, mock_emit, _mock_on_commit): @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) + now = datetime.now() + with patch.object(EditInfoMixin, 'subtree_edited_on', now): + sh.emit_catalog_info_changed_signal(self.course_key) + mock_signal.send_event.assert_called_once_with( + time=now.replace(tzinfo=timezone.utc), + catalog_info=self.expected_data) @override_settings(SEND_CATALOG_INFO_SIGNAL=True) @patch('cms.djangoapps.contentstore.signals.handlers.COURSE_CATALOG_INFO_CHANGED', autospec=True)