Skip to content

Commit

Permalink
feat!: use mongo timestamp when sending course_catalog_info_changed e…
Browse files Browse the repository at this point in the history
…vents 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 <rraposa@edx.org>
  • Loading branch information
Rebecca Graber and robrap authored Feb 3, 2023
1 parent ec70ac7 commit a8598fa
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
17 changes: 10 additions & 7 deletions cms/djangoapps/contentstore/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@


import logging
from datetime import datetime
from datetime import datetime, timezone
from functools import wraps
from typing import Optional

Expand Down Expand Up @@ -66,24 +66,27 @@ 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.
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.
(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(
Expand All @@ -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)
Expand Down
11 changes: 8 additions & 3 deletions cms/djangoapps/contentstore/signals/tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
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
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.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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit a8598fa

Please sign in to comment.