From 2c7c37d05f0232130836e9092714dba20d5d90ef Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 1 Nov 2023 11:30:20 -0400 Subject: [PATCH 01/11] feat: remove manual sends of events --- .../contentstore/signals/handlers.py | 89 +------------------ common/djangoapps/student/handlers.py | 34 ------- lms/djangoapps/certificates/signals.py | 68 -------------- 3 files changed, 1 insertion(+), 190 deletions(-) delete mode 100644 common/djangoapps/student/handlers.py diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 88458742612f..12687bc13bc6 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -13,14 +13,7 @@ from edx_toggles.toggles import SettingToggle from opaque_keys.edx.keys import CourseKey from openedx_events.content_authoring.data import CourseCatalogData, CourseScheduleData -from openedx_events.content_authoring.signals import ( - COURSE_CATALOG_INFO_CHANGED, - XBLOCK_DELETED, - XBLOCK_DUPLICATED, - XBLOCK_PUBLISHED, -) -from openedx.core.lib.events import determine_producer_config_for_signal_and_topic -from openedx_events.event_bus import get_producer +from openedx_events.content_authoring.signals import COURSE_CATALOG_INFO_CHANGED from pytz import UTC from cms.djangoapps.contentstore.courseware_index import ( @@ -159,86 +152,6 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Send to a signal for catalog info changes as well, but only once we know the transaction is committed. transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) - -@receiver(COURSE_CATALOG_INFO_CHANGED) -def listen_for_course_catalog_info_changed(sender, signal, **kwargs): - """ - Publish COURSE_CATALOG_INFO_CHANGED signals onto the event bus. - """ - # temporary: defer to EVENT_BUS_PRODUCER_CONFIG if present - producer_config_setting = determine_producer_config_for_signal_and_topic(COURSE_CATALOG_INFO_CHANGED, - 'course-catalog-info-changed') - if producer_config_setting is True: - log.info("Producing course-catalog-info-changed event via config") - return - log.info("Producing course-catalog-info-changed event via manual send") - get_producer().send( - signal=COURSE_CATALOG_INFO_CHANGED, topic='course-catalog-info-changed', - event_key_field='catalog_info.course_key', event_data={'catalog_info': kwargs['catalog_info']}, - event_metadata=kwargs['metadata'], - ) - - -@receiver(XBLOCK_PUBLISHED) -def listen_for_xblock_published(sender, signal, **kwargs): - """ - Publish XBLOCK_PUBLISHED signals onto the event bus. - """ - # temporary: defer to EVENT_BUS_PRODUCER_CONFIG if present - topic = getattr(settings, "EVENT_BUS_XBLOCK_LIFECYCLE_TOPIC", "course-authoring-xblock-lifecycle") - producer_config_setting = determine_producer_config_for_signal_and_topic(XBLOCK_PUBLISHED, topic) - if producer_config_setting is True: - log.info("Producing xblock-published event via config") - return - if settings.FEATURES.get("ENABLE_SEND_XBLOCK_EVENTS_OVER_BUS"): - log.info("Producing xblock-published event via manual send") - get_producer().send( - signal=XBLOCK_PUBLISHED, topic=topic, - event_key_field='xblock_info.usage_key', event_data={'xblock_info': kwargs['xblock_info']}, - event_metadata=kwargs['metadata'], - ) - - -@receiver(XBLOCK_DELETED) -def listen_for_xblock_deleted(sender, signal, **kwargs): - """ - Publish XBLOCK_DELETED signals onto the event bus. - """ - # temporary: defer to EVENT_BUS_PRODUCER_CONFIG if present - topic = getattr(settings, "EVENT_BUS_XBLOCK_LIFECYCLE_TOPIC", "course-authoring-xblock-lifecycle") - producer_config_setting = determine_producer_config_for_signal_and_topic(XBLOCK_DELETED, topic) - if producer_config_setting is True: - log.info("Producing xblock-deleted event via config") - return - if settings.FEATURES.get("ENABLE_SEND_XBLOCK_EVENTS_OVER_BUS"): - log.info("Producing xblock-deleted event via manual send") - get_producer().send( - signal=XBLOCK_DELETED, topic=topic, - event_key_field='xblock_info.usage_key', event_data={'xblock_info': kwargs['xblock_info']}, - event_metadata=kwargs['metadata'], - ) - - -@receiver(XBLOCK_DUPLICATED) -def listen_for_xblock_duplicated(sender, signal, **kwargs): - """ - Publish XBLOCK_DUPLICATED signals onto the event bus. - """ - # temporary: defer to EVENT_BUS_PRODUCER_CONFIG if present - topic = getattr(settings, "EVENT_BUS_XBLOCK_LIFECYCLE_TOPIC", "course-authoring-xblock-lifecycle") - producer_config_setting = determine_producer_config_for_signal_and_topic(XBLOCK_DUPLICATED, topic) - if producer_config_setting is True: - log.info("Producing xblock-duplicated event via config") - return - if settings.FEATURES.get("ENABLE_SEND_XBLOCK_EVENTS_OVER_BUS"): - log.info("Producing xblock-duplicated event via manual send") - get_producer().send( - signal=XBLOCK_DUPLICATED, topic=topic, - event_key_field='xblock_info.usage_key', event_data={'xblock_info': kwargs['xblock_info']}, - event_metadata=kwargs['metadata'], - ) - - @receiver(SignalHandler.course_deleted) def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument """ diff --git a/common/djangoapps/student/handlers.py b/common/djangoapps/student/handlers.py deleted file mode 100644 index d7d5eeee3ce0..000000000000 --- a/common/djangoapps/student/handlers.py +++ /dev/null @@ -1,34 +0,0 @@ -""" -Handlers for student -""" -from django.conf import settings -from django.dispatch import receiver - -from openedx_events.event_bus import get_producer -from openedx_events.learning.signals import ( - COURSE_UNENROLLMENT_COMPLETED, -) -from openedx.core.lib.events import determine_producer_config_for_signal_and_topic -import logging -log = logging.getLogger(__name__) - - -@receiver(COURSE_UNENROLLMENT_COMPLETED) -def course_unenrollment_receiver(sender, signal, **kwargs): - """ - Removes user notification preference when user un-enrolls from the course - """ - topic = getattr(settings, "EVENT_BUS_ENROLLMENT_LIFECYCLE_TOPIC", "course-unenrollment-lifecycle") - producer_config_setting = determine_producer_config_for_signal_and_topic(COURSE_UNENROLLMENT_COMPLETED, topic) - if producer_config_setting is True: - log.info("Producing unenrollment-event event via config") - return - if settings.FEATURES.get("ENABLE_SEND_ENROLLMENT_EVENTS_OVER_BUS"): - log.info("Producing unenrollment-event event via manual send") - get_producer().send( - signal=COURSE_UNENROLLMENT_COMPLETED, - topic=topic, - event_key_field='enrollment.course.course_key', - event_data={'enrollment': kwargs.get('enrollment')}, - event_metadata=kwargs.get('metadata') - ) diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index b6858c144597..ed7c6685fc63 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -162,71 +162,3 @@ def _listen_for_enrollment_mode_change(sender, user, course_key, mode, **kwargs) course_key, ) return False - - -def _determine_producer_config_for_signal_and_topic(signal, topic): - """ - Utility method to determine the setting for the given signal and topic in EVENT_BUS_PRODUCER_CONFIG - - Records to New Relic for later analysis. - - Parameters - signal (OpenEdxPublicSignal): The signal being sent to the event bus - topic (string): The topic to which the signal is being sent (without environment prefix) - - Returns - True if the signal is enabled for that topic in EVENT_BUS_PRODUCER_CONFIG - False if the signal is explicitly disabled for that topic in EVENT_BUS_PRODUCER_CONFIG - None if the signal/topic pair is not present in EVENT_BUS_PRODUCER_CONFIG - """ - event_type_producer_configs = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", - {}).get(signal.event_type, {}) - topic_config = event_type_producer_configs.get(topic, {}) - topic_setting = topic_config.get('enabled', None) - set_custom_attribute(f'producer_config_setting_{topic}_{signal.event_type}', - topic_setting if topic_setting is not None else 'Unset') - return topic_setting - - -@receiver(CERTIFICATE_CREATED) -def listen_for_certificate_created_event(sender, signal, **kwargs): # pylint: disable=unused-argument - """ - Publish `CERTIFICATE_CREATED` events to the event bus. - """ - # temporary: defer to EVENT_BUS_PRODUCER_CONFIG if present - producer_config_setting = determine_producer_config_for_signal_and_topic(CERTIFICATE_CREATED, - 'learning-certificate-lifecycle') - if producer_config_setting is True: - log.info("Producing certificate-created event via config") - return - if SEND_CERTIFICATE_CREATED_SIGNAL.is_enabled(): - log.info("Producing certificate-created event via manual send") - get_producer().send( - signal=CERTIFICATE_CREATED, - topic='learning-certificate-lifecycle', - event_key_field='certificate.course.course_key', - event_data={'certificate': kwargs['certificate']}, - event_metadata=kwargs['metadata'] - ) - - -@receiver(CERTIFICATE_REVOKED) -def listen_for_certificate_revoked_event(sender, signal, **kwargs): # pylint: disable=unused-argument - """ - Publish `CERTIFICATE_REVOKED` events to the event bus. - """ - # temporary: defer to EVENT_BUS_PRODUCER_CONFIG if present - producer_config_setting = determine_producer_config_for_signal_and_topic(CERTIFICATE_REVOKED, - 'learning-certificate-lifecycle') - if producer_config_setting is True: - log.info("Producing certificate-revoked event via config") - return - if SEND_CERTIFICATE_REVOKED_SIGNAL.is_enabled(): - log.info("Producing certificate-revoked event via manual send") - get_producer().send( - signal=CERTIFICATE_REVOKED, - topic='learning-certificate-lifecycle', - event_key_field='certificate.course.course_key', - event_data={'certificate': kwargs['certificate']}, - event_metadata=kwargs['metadata'] - ) From 676db03c7734ca09427804c3534fe7c8b448a4f9 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 13:22:53 -0400 Subject: [PATCH 02/11] feat: remove manual sends of events --- cms/envs/devstack.py | 20 ++++++------------ lms/djangoapps/certificates/config.py | 28 -------------------------- lms/djangoapps/certificates/signals.py | 5 ----- lms/envs/devstack.py | 2 ++ 4 files changed, 8 insertions(+), 47 deletions(-) diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 29c77701886e..a874313b8e23 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -301,24 +301,16 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:18150' #################### Event bus backend ######################## -# .. toggle_name: FEATURES['ENABLE_SEND_XBLOCK_EVENTS_OVER_BUS'] -# .. toggle_implementation: DjangoSetting -# .. toggle_default: False -# .. toggle_description: Temporary configuration which enables sending xblock events over the event bus. -# .. toggle_use_cases: open_edx -# .. toggle_creation_date: 2023-02-21 -# .. toggle_warning: For consistency in user experience, keep the value in sync with the setting of the same name -# in the LMS and CMS. -# This will be deprecated in favor of ENABLE_SEND_XBLOCK_LIFECYCLE_EVENTS_OVER_BUS -# .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/31813' -FEATURES['ENABLE_SEND_XBLOCK_EVENTS_OVER_BUS'] = True -FEATURES['ENABLE_SEND_ENROLLMENT_EVENTS_OVER_BUS'] = True + EVENT_BUS_PRODUCER = 'edx_event_bus_redis.create_producer' EVENT_BUS_REDIS_CONNECTION_URL = 'redis://:password@edx.devstack.redis:6379/' EVENT_BUS_TOPIC_PREFIX = 'dev' EVENT_BUS_CONSUMER = 'edx_event_bus_redis.RedisEventConsumer' -EVENT_BUS_XBLOCK_LIFECYCLE_TOPIC = 'course-authoring-xblock-lifecycle' -EVENT_BUS_ENROLLMENT_LIFECYCLE_TOPIC = 'course-authoring-enrollment-lifecycle' +EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.course.catalog_info.changed.v1']['course-catalog-info-changed']['enabled']=True +EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.published.v1']['course-authoring-xblock-lifecycle']['enabled']=True +EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.deleted.v1']['course-authoring-xblock-lifecycle']['enabled']=True +EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.duplicated.v1']['course-authoring-xblock-lifecycle']['enabled']=True + ################# New settings must go ABOVE this line ################# ######################################################################## diff --git a/lms/djangoapps/certificates/config.py b/lms/djangoapps/certificates/config.py index 92b20b17bb27..10caa1338921 100644 --- a/lms/djangoapps/certificates/config.py +++ b/lms/djangoapps/certificates/config.py @@ -15,31 +15,3 @@ # .. toggle_use_cases: open_edx # .. toggle_creation_date: 2017-09-14 AUTO_CERTIFICATE_GENERATION = WaffleSwitch(f"{WAFFLE_NAMESPACE}.auto_certificate_generation", __name__) - - -# .. toggle_name: SEND_CERTIFICATE_CREATED_SIGNAL -# .. toggle_implementation: SettingToggle -# .. toggle_default: False -# .. toggle_description: When True, the system will publish `CERTIFICATE_CREATED` signals to the event bus. The -# `CERTIFICATE_CREATED` signal is emit when a certificate has been awarded to a learner and the creation process has -# completed. -# .. toggle_warning: Will be deprecated in favor of SEND_LEARNING_CERTIFICATE_LIFECYCLE_EVENTS_TO_BUS -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2023-04-11 -# .. toggle_target_removal_date: 2023-07-31 -# .. toggle_tickets: TODO -SEND_CERTIFICATE_CREATED_SIGNAL = SettingToggle('SEND_CERTIFICATE_CREATED_SIGNAL', default=False, module_name=__name__) - - -# .. toggle_name: SEND_CERTIFICATE_REVOKED_SIGNAL -# .. toggle_implementation: SettingToggle -# .. toggle_default: False -# .. toggle_description: When True, the system will publish `CERTIFICATE_REVOKED` signals to the event bus. The -# `CERTIFICATE_REVOKED` signal is emit when a certificate has been revoked from a learner and the revocation process -# has completed. -# .. toggle_warning: Will be deprecated in favor of SEND_LEARNING_CERTIFICATE_LIFECYCLE_EVENTS_TO_BUS -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2023-09-15 -# .. toggle_target_removal_date: 2024-01-01 -# .. toggle_tickets: TODO -SEND_CERTIFICATE_REVOKED_SIGNAL = SettingToggle('SEND_CERTIFICATE_REVOKED_SIGNAL', default=False, module_name=__name__) diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index ed7c6685fc63..30e211f58a1a 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -7,13 +7,10 @@ from django.db.models.signals import post_save from django.dispatch import receiver -from openedx_events.event_bus import get_producer -from edx_django_utils.monitoring import set_custom_attribute from common.djangoapps.course_modes import api as modes_api from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.signals import ENROLLMENT_TRACK_UPDATED -from lms.djangoapps.certificates.config import SEND_CERTIFICATE_CREATED_SIGNAL, SEND_CERTIFICATE_REVOKED_SIGNAL from lms.djangoapps.certificates.generation_handler import ( CertificateGenerationNotAllowed, generate_allowlist_certificate_task, @@ -29,13 +26,11 @@ from lms.djangoapps.certificates.api import auto_certificate_generation_enabled from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.content.course_overviews.signals import COURSE_PACING_CHANGED -from openedx.core.lib.events import determine_producer_config_for_signal_and_topic from openedx.core.djangoapps.signals.signals import ( COURSE_GRADE_NOW_FAILED, COURSE_GRADE_NOW_PASSED, LEARNER_NOW_VERIFIED ) -from openedx_events.learning.signals import CERTIFICATE_CREATED, CERTIFICATE_REVOKED log = logging.getLogger(__name__) diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 4eae239aabbe..89aba7e0cfb1 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -508,6 +508,8 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing EVENT_BUS_REDIS_CONNECTION_URL = 'redis://:password@edx.devstack.redis:6379/' EVENT_BUS_TOPIC_PREFIX = 'dev' EVENT_BUS_CONSUMER = 'edx_event_bus_redis.RedisEventConsumer' +EVENT_BUS_PRODUCER_CONFIG['org.openedx.learning.certificate.revoked.v1']['learning-certificate-lifecycle']['enabled']=True +EVENT_BUS_PRODUCER_CONFIG['org.openedx.learning.certificate.created.v1']['learning-certificate-lifecycle']['enabled']=True ######################## Subscriptions API SETTINGS ######################## SUBSCRIPTIONS_ROOT_URL = "http://host.docker.internal:18750" From 5d84a4dbffa4df15bed3876d0aaea510615cac93 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 13:26:33 -0400 Subject: [PATCH 03/11] fixup!: rm unused file --- openedx/core/lib/events.py | 28 ---------------------------- 1 file changed, 28 deletions(-) delete mode 100644 openedx/core/lib/events.py diff --git a/openedx/core/lib/events.py b/openedx/core/lib/events.py deleted file mode 100644 index bc90824d5119..000000000000 --- a/openedx/core/lib/events.py +++ /dev/null @@ -1,28 +0,0 @@ -"""Temporary method for use in rolling out a new event producer configuration.""" - -from django.conf import settings -from edx_django_utils.monitoring import set_custom_attribute - - -def determine_producer_config_for_signal_and_topic(signal, topic): - """ - Utility method to determine the setting for the given signal and topic in EVENT_BUS_PRODUCER_CONFIG - - Records to New Relic for later analysis. - - Parameters - signal (OpenEdxPublicSignal): The signal being sent to the event bus - topic (string): The topic to which the signal is being sent (without environment prefix) - - Returns - True if the signal is enabled for that topic in EVENT_BUS_PRODUCER_CONFIG - False if the signal is explicitly disabled for that topic in EVENT_BUS_PRODUCER_CONFIG - None if the signal/topic pair is not present in EVENT_BUS_PRODUCER_CONFIG - """ - event_type_producer_configs = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", - {}).get(signal.event_type, {}) - topic_config = event_type_producer_configs.get(topic, {}) - topic_setting = topic_config.get('enabled', None) - set_custom_attribute(f'producer_config_setting_{topic}_{signal.event_type}', - topic_setting if topic_setting is not None else 'Unset') - return topic_setting From a513fb56ea511258bf914565086822bf8ca18224 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 13:43:58 -0400 Subject: [PATCH 04/11] fixup!: delete unneeded tests --- .../djangoapps/student/tests/test_handlers.py | 67 ----------- .../certificates/tests/test_signals.py | 110 ------------------ 2 files changed, 177 deletions(-) delete mode 100644 common/djangoapps/student/tests/test_handlers.py diff --git a/common/djangoapps/student/tests/test_handlers.py b/common/djangoapps/student/tests/test_handlers.py deleted file mode 100644 index bd8d6a9baf47..000000000000 --- a/common/djangoapps/student/tests/test_handlers.py +++ /dev/null @@ -1,67 +0,0 @@ -""" -Unit tests for event bus tests for course unenrollments -""" - -import unittest -from datetime import datetime, timezone -from unittest import mock -from uuid import uuid4 - -from django.test.utils import override_settings -from common.djangoapps.student.handlers import course_unenrollment_receiver -from common.djangoapps.student.tests.factories import ( - UserFactory, - CourseEnrollmentFactory, -) - -from openedx_events.data import EventsMetadata -from openedx_events.learning.signals import COURSE_UNENROLLMENT_COMPLETED -from pytest import mark - - -@mark.django_db -class UnenrollmentEventBusTests(unittest.TestCase): - """ - Tests for unenrollment events that interact with the event bus. - """ - @override_settings(ENABLE_SEND_ENROLLMENT_EVENTS_OVER_BUS=False) - @mock.patch('common.djangoapps.student.handlers.get_producer', autospec=True) - def test_event_disabled(self, mock_producer): - """ - Test to verify that we do not push `CERTIFICATE_CREATED` events to the event bus if the - `SEND_CERTIFICATE_CREATED_SIGNAL` setting is disabled. - """ - course_unenrollment_receiver(None, None) - mock_producer.assert_not_called() - - @override_settings(FEATURES={'ENABLE_SEND_ENROLLMENT_EVENTS_OVER_BUS': True}) - @mock.patch('common.djangoapps.student.handlers.get_producer', autospec=True) - def test_event_enabled(self, mock_producer): - """ - Test to verify that we push `COURSE_UNENROLLMENT_COMPLETED` events to the event bus. - """ - user = UserFactory() - enrollment = CourseEnrollmentFactory(user=user) - - event_metadata = EventsMetadata( - event_type=COURSE_UNENROLLMENT_COMPLETED.event_type, - id=uuid4(), - minorversion=0, - source='openedx/lms/web', - sourcehost='lms.test', - time=datetime.now(timezone.utc) - ) - - event_kwargs = { - 'enrollment': enrollment, - 'metadata': event_metadata - } - course_unenrollment_receiver(None, COURSE_UNENROLLMENT_COMPLETED, **event_kwargs) - - # verify that the data sent to the event bus matches what we expect - print(mock_producer.return_value) - print(mock_producer.return_value.send.call_args) - data = mock_producer.return_value.send.call_args.kwargs - assert data['event_data']['enrollment'] == enrollment - assert data['topic'] == 'course-unenrollment-lifecycle' - assert data['event_key_field'] == 'enrollment.course.course_key' diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 4eb8cf27f48e..6f4c888c6832 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -21,10 +21,6 @@ CertificateGenerationConfiguration, GeneratedCertificate ) -from lms.djangoapps.certificates.signals import ( - listen_for_certificate_created_event, - listen_for_certificate_revoked_event -) from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory, GeneratedCertificateFactory from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.tests.utils import mock_passing_grade @@ -443,109 +439,3 @@ def test_verified_to_audit(self): ) as mock_allowlist_task: self.verified_enrollment.change_mode('audit') mock_allowlist_task.assert_not_called() - - -class CertificateEventBusTests(ModuleStoreTestCase): - """ - Tests for Certificate events that interact with the event bus. - """ - def setUp(self): - super().setUp() - self.user = UserFactory.create() - self.name = f'{self.user.first_name} {self.user.last_name}' - self.course = CourseFactory.create(self_paced=True) - self.enrollment = CourseEnrollmentFactory( - user=self.user, - course_id=self.course.id, - is_active=True, - mode='verified', - ) - - def _create_event_data(self, event_type, certificate_status): - """ - Utility function to create test data for unit tests. - """ - expected_course_data = CourseData(course_key=self.course.id) - expected_user_data = UserData( - pii=UserPersonalData( - username=self.user.username, - email=self.user.email, - name=self.name, - ), - id=self.user.id, - is_active=self.user.is_active - ) - expected_certificate_data = CertificateData( - user=expected_user_data, - course=expected_course_data, - mode='verified', - grade='', - current_status=certificate_status, - download_url='', - name='', - ) - expected_event_metadata = EventsMetadata( - event_type=event_type.event_type, - id=uuid4(), - minorversion=0, - source='openedx/lms/web', - sourcehost='lms.test', - time=datetime.now(timezone.utc) - ) - - return { - 'certificate': expected_certificate_data, - 'metadata': expected_event_metadata, - } - - @override_settings(SEND_CERTIFICATE_CREATED_SIGNAL=False) - @mock.patch('lms.djangoapps.certificates.signals.get_producer', autospec=True) - def test_certificate_created_event_disabled(self, mock_producer): - """ - Test to verify that we do not publish `CERTIFICATE_CREATED` events to the event bus if the - `SEND_CERTIFICATE_CREATED_SIGNAL` setting is disabled. - """ - listen_for_certificate_created_event(None, CERTIFICATE_CREATED) - mock_producer.assert_not_called() - - @override_settings(SEND_CERTIFICATE_REVOKED_SIGNAL=False) - @mock.patch('lms.djangoapps.certificates.signals.get_producer', autospec=True) - def test_certificate_revoked_event_disabled(self, mock_producer): - """ - Test to verify that we do not publish `CERTIFICATE_REVOKED` events to the event bus if the - `SEND_CERTIFICATE_REVOKED_SIGNAL` setting is disabled. - """ - listen_for_certificate_created_event(None, CERTIFICATE_REVOKED) - mock_producer.assert_not_called() - - @override_settings(SEND_CERTIFICATE_CREATED_SIGNAL=True) - @mock.patch('lms.djangoapps.certificates.signals.get_producer', autospec=True) - def test_certificate_created_event_enabled(self, mock_producer): - """ - Test to verify that we push `CERTIFICATE_CREATED` events to the event bus if the - `SEND_CERTIFICATE_CREATED_SIGNAL` setting is enabled. - """ - event_data = self._create_event_data(CERTIFICATE_CREATED, CertificateStatuses.downloadable) - listen_for_certificate_created_event(None, CERTIFICATE_CREATED, **event_data) - # verify that the data sent to the event bus matches what we expect - data = mock_producer.return_value.send.call_args.kwargs - assert data['signal'].event_type == CERTIFICATE_CREATED.event_type - assert data['event_data']['certificate'] == event_data['certificate'] - assert data['topic'] == 'learning-certificate-lifecycle' - assert data['event_key_field'] == 'certificate.course.course_key' - - @override_settings(SEND_CERTIFICATE_REVOKED_SIGNAL=True) - @mock.patch('lms.djangoapps.certificates.signals.get_producer', autospec=True) - def test_certificate_revoked_event_enabled(self, mock_producer): - """ - Test to verify that we push `CERTIFICATE_REVOKED` events to the event bus if the - `SEND_CERTIFICATE_REVOKED_SIGNAL` setting is enabled. - """ - event_data = self._create_event_data(CERTIFICATE_REVOKED, CertificateStatuses.notpassing) - listen_for_certificate_revoked_event(None, CERTIFICATE_REVOKED, **event_data) - # verify that the data sent to the event bus matches what we expect - data = mock_producer.return_value.send.call_args.kwargs - assert data['signal'].event_type == CERTIFICATE_REVOKED.event_type - assert data['event_data']['certificate'] == event_data['certificate'] - assert data['topic'] == 'learning-certificate-lifecycle' - assert data['event_key_field'] == 'certificate.course.course_key' From 34bfa5a777202f59c894e65e24e8e190de78709e Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 13:44:40 -0400 Subject: [PATCH 05/11] fixup!: rm unneeded imports --- lms/djangoapps/certificates/tests/test_signals.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 6f4c888c6832..c8a9d2a35306 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -25,9 +25,6 @@ from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.tests.utils import mock_passing_grade from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification -from openedx_events.data import EventsMetadata -from openedx_events.learning.signals import CERTIFICATE_CREATED, CERTIFICATE_REVOKED -from openedx_events.learning.data import CourseData, UserData, UserPersonalData, CertificateData class SelfGeneratedCertsSignalTest(ModuleStoreTestCase): From e0ae670a20e7446dd1678cecab81aa15580fa283 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 14:17:51 -0400 Subject: [PATCH 06/11] fixup!: quality --- cms/envs/devstack.py | 21 +++++++++++++++++---- lms/envs/devstack.py | 12 ++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index a874313b8e23..0f153e391e46 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -306,11 +306,24 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing EVENT_BUS_REDIS_CONNECTION_URL = 'redis://:password@edx.devstack.redis:6379/' EVENT_BUS_TOPIC_PREFIX = 'dev' EVENT_BUS_CONSUMER = 'edx_event_bus_redis.RedisEventConsumer' -EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.course.catalog_info.changed.v1']['course-catalog-info-changed']['enabled']=True -EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.published.v1']['course-authoring-xblock-lifecycle']['enabled']=True -EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.deleted.v1']['course-authoring-xblock-lifecycle']['enabled']=True -EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.duplicated.v1']['course-authoring-xblock-lifecycle']['enabled']=True +# use 'get' instead of [] access to break up long lines better +EVENT_BUS_PRODUCER_CONFIG\ + .get('org.openedx.content_authoring.course.catalog_info.changed.v1')\ + .get('course-catalog-info-changed')\ + .set('enabled', True) +EVENT_BUS_PRODUCER_CONFIG\ + .get('org.openedx.content_authoring.xblock.published.v1')\ + .get(['course-authoring-xblock-lifecycle'])\ + .set('enabled', True) +EVENT_BUS_PRODUCER_CONFIG\ + .get('org.openedx.content_authoring.xblock.deleted.v1')\ + .get('course-authoring-xblock-lifecycle')\ + .set('enabled', True) +EVENT_BUS_PRODUCER_CONFIG\ + .get('org.openedx.content_authoring.xblock.duplicated.v1')\ + .get('course-authoring-xblock-lifecycle')\ + .set('enabled', True) ################# New settings must go ABOVE this line ################# ######################################################################## diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 89aba7e0cfb1..8ac84a29cc1f 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -508,8 +508,16 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing EVENT_BUS_REDIS_CONNECTION_URL = 'redis://:password@edx.devstack.redis:6379/' EVENT_BUS_TOPIC_PREFIX = 'dev' EVENT_BUS_CONSUMER = 'edx_event_bus_redis.RedisEventConsumer' -EVENT_BUS_PRODUCER_CONFIG['org.openedx.learning.certificate.revoked.v1']['learning-certificate-lifecycle']['enabled']=True -EVENT_BUS_PRODUCER_CONFIG['org.openedx.learning.certificate.created.v1']['learning-certificate-lifecycle']['enabled']=True + +# use 'get' instead of [] access to break up long lines better +EVENT_BUS_PRODUCER_CONFIG\ + .get('org.openedx.learning.certificate.revoked.v1')\ + .get('learning-certificate-lifecycle')\ + .set('enabled', True) +EVENT_BUS_PRODUCER_CONFIG\ + .get('org.openedx.learning.certificate.created.v1')\ + .get('learning-certificate-lifecycle')\ + .set('enabled', True) ######################## Subscriptions API SETTINGS ######################## SUBSCRIPTIONS_ROOT_URL = "http://host.docker.internal:18750" From b28cf4433279bead7078beea38a4fff0b4faf344 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 15:01:31 -0400 Subject: [PATCH 07/11] fixup!: ugh --- cms/envs/devstack.py | 27 ++++++++++----------------- lms/envs/devstack.py | 13 ++++--------- 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 0f153e391e46..674416c10f7d 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -307,23 +307,16 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing EVENT_BUS_TOPIC_PREFIX = 'dev' EVENT_BUS_CONSUMER = 'edx_event_bus_redis.RedisEventConsumer' -# use 'get' instead of [] access to break up long lines better -EVENT_BUS_PRODUCER_CONFIG\ - .get('org.openedx.content_authoring.course.catalog_info.changed.v1')\ - .get('course-catalog-info-changed')\ - .set('enabled', True) -EVENT_BUS_PRODUCER_CONFIG\ - .get('org.openedx.content_authoring.xblock.published.v1')\ - .get(['course-authoring-xblock-lifecycle'])\ - .set('enabled', True) -EVENT_BUS_PRODUCER_CONFIG\ - .get('org.openedx.content_authoring.xblock.deleted.v1')\ - .get('course-authoring-xblock-lifecycle')\ - .set('enabled', True) -EVENT_BUS_PRODUCER_CONFIG\ - .get('org.openedx.content_authoring.xblock.duplicated.v1')\ - .get('course-authoring-xblock-lifecycle')\ - .set('enabled', True) +course_catalog_event_setting = EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.course.catalog_info.changed.v1'] +course_catalog_event_setting['course-catalog-info-changed']['enabled'] = True + +xblock_published_event_setting = EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.published.v1'] +xblock_published_event_setting['course-authoring-xblock-lifecycle']['enabled'] = True +xblock_deleted_event_setting = EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.deleted.v1'] +xblock_deleted_event_setting['course-authoring-xblock-lifecycle']['enabled'] = True +xblock_duplicated_event_setting = EVENT_BUS_PRODUCER_CONFIG['org.openedx.content_authoring.xblock.duplicated.v1'] +xblock_duplicated_event_setting['course-authoring-xblock-lifecycle']['enabled'] = True + ################# New settings must go ABOVE this line ################# ######################################################################## diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 8ac84a29cc1f..4a262c7c8bef 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -509,15 +509,10 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing EVENT_BUS_TOPIC_PREFIX = 'dev' EVENT_BUS_CONSUMER = 'edx_event_bus_redis.RedisEventConsumer' -# use 'get' instead of [] access to break up long lines better -EVENT_BUS_PRODUCER_CONFIG\ - .get('org.openedx.learning.certificate.revoked.v1')\ - .get('learning-certificate-lifecycle')\ - .set('enabled', True) -EVENT_BUS_PRODUCER_CONFIG\ - .get('org.openedx.learning.certificate.created.v1')\ - .get('learning-certificate-lifecycle')\ - .set('enabled', True) +certificate_revoked_event_config = EVENT_BUS_PRODUCER_CONFIG['org.openedx.learning.certificate.revoked.v1'] +certificate_revoked_event_config['learning-certificate-lifecycle']['enabled'] = True +certificate_created_event_config = EVENT_BUS_PRODUCER_CONFIG['org.openedx.learning.certificate.created.v1'] +certificate_created_event_config['learning-certificate-lifecycle']['enabled'] = True ######################## Subscriptions API SETTINGS ######################## SUBSCRIPTIONS_ROOT_URL = "http://host.docker.internal:18750" From b83ff72867b01fbb62beccfd94ede9751dd98b89 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 15:18:59 -0400 Subject: [PATCH 08/11] fixup!: still more lint --- cms/djangoapps/contentstore/signals/handlers.py | 1 + lms/djangoapps/certificates/config.py | 2 -- lms/djangoapps/certificates/signals.py | 1 - lms/djangoapps/certificates/tests/test_signals.py | 3 --- 4 files changed, 1 insertion(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 12687bc13bc6..20c14089e0a6 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -152,6 +152,7 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Send to a signal for catalog info changes as well, but only once we know the transaction is committed. transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) + @receiver(SignalHandler.course_deleted) def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument """ diff --git a/lms/djangoapps/certificates/config.py b/lms/djangoapps/certificates/config.py index 10caa1338921..3a835cd020d3 100644 --- a/lms/djangoapps/certificates/config.py +++ b/lms/djangoapps/certificates/config.py @@ -3,8 +3,6 @@ waffle switches for the Certificates app. """ -from edx_toggles.toggles import SettingToggle, WaffleSwitch - # Namespace WAFFLE_NAMESPACE = 'certificates' diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 30e211f58a1a..28e68e340253 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -3,7 +3,6 @@ """ import logging -from django.conf import settings from django.db.models.signals import post_save from django.dispatch import receiver diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index c8a9d2a35306..320eec4d61d0 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -3,12 +3,9 @@ and disabling for instructor-paced courses. """ -from datetime import datetime, timezone from unittest import mock -from uuid import uuid4 import ddt -from django.test.utils import override_settings from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory From c29a257a43003ab65d79fa7fbf72b1e21ededf23 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 15:23:28 -0400 Subject: [PATCH 09/11] fixup!: oops bad delete --- lms/djangoapps/certificates/config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/certificates/config.py b/lms/djangoapps/certificates/config.py index 3a835cd020d3..1a5fd387581b 100644 --- a/lms/djangoapps/certificates/config.py +++ b/lms/djangoapps/certificates/config.py @@ -2,6 +2,7 @@ This module contains various configuration settings via waffle switches for the Certificates app. """ +from edx_toggles.toggles import WaffleSwitch # Namespace WAFFLE_NAMESPACE = 'certificates' From db144385f94b9d368f895c514473b5a40aa685b7 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 15:23:52 -0400 Subject: [PATCH 10/11] fixup!: too many spaces --- cms/djangoapps/contentstore/signals/handlers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 20c14089e0a6..12687bc13bc6 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -152,7 +152,6 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Send to a signal for catalog info changes as well, but only once we know the transaction is committed. transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) - @receiver(SignalHandler.course_deleted) def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument """ From 284ee04d0b3f432c74bd3e5c551e99a043039308 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 2 Nov 2023 15:36:10 -0400 Subject: [PATCH 11/11] fixup!: not enough spaces --- cms/djangoapps/contentstore/signals/handlers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 12687bc13bc6..20c14089e0a6 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -152,6 +152,7 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Send to a signal for catalog info changes as well, but only once we know the transaction is committed. transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) + @receiver(SignalHandler.course_deleted) def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument """