From b4f265ade21e765e202959a149b5b1efb686261b Mon Sep 17 00:00:00 2001 From: Gregory Martin Date: Tue, 16 May 2017 11:32:41 -0400 Subject: [PATCH] Refactor CMS Signal Handling https://openedx.atlassian.net/browse/EDUCATOR-389 --- cms/djangoapps/contentstore/apps.py | 22 +++++++++++++++++++ .../contentstore/signals/__init__.py | 0 .../{signals.py => signals/handlers.py} | 18 +++------------ .../contentstore/signals/signals.py | 14 ++++++++++++ cms/djangoapps/contentstore/startup.py | 2 -- .../tests/test_courseware_index.py | 2 +- .../contentstore/tests/test_gating.py | 10 ++++----- .../contentstore/tests/test_proctoring.py | 2 +- .../contentstore/views/tests/test_gating.py | 4 ++-- cms/envs/common.py | 3 ++- 10 files changed, 50 insertions(+), 27 deletions(-) create mode 100644 cms/djangoapps/contentstore/apps.py create mode 100644 cms/djangoapps/contentstore/signals/__init__.py rename cms/djangoapps/contentstore/{signals.py => signals/handlers.py} (86%) create mode 100644 cms/djangoapps/contentstore/signals/signals.py delete mode 100644 cms/djangoapps/contentstore/startup.py diff --git a/cms/djangoapps/contentstore/apps.py b/cms/djangoapps/contentstore/apps.py new file mode 100644 index 0000000000..676843d064 --- /dev/null +++ b/cms/djangoapps/contentstore/apps.py @@ -0,0 +1,22 @@ +""" +Contentstore Application Configuration + +Above-modulestore level signal handlers are connected here. +""" + +from django.apps import AppConfig + + +class ContentstoreConfig(AppConfig): + """ + Application Configuration for Grades. + """ + name = u'contentstore' + + def ready(self): + """ + Connect handlers to signals. + """ + # Can't import models at module level in AppConfigs, and models get + # included from the signal handlers + from .signals import handlers # pylint: disable=unused-variable diff --git a/cms/djangoapps/contentstore/signals/__init__.py b/cms/djangoapps/contentstore/signals/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/contentstore/signals.py b/cms/djangoapps/contentstore/signals/handlers.py similarity index 86% rename from cms/djangoapps/contentstore/signals.py rename to cms/djangoapps/contentstore/signals/handlers.py index cf0806614e..2742832420 100644 --- a/cms/djangoapps/contentstore/signals.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -4,7 +4,7 @@ import logging from datetime import datetime from pytz import UTC -from django.dispatch import receiver, Signal +from django.dispatch import receiver from xmodule.modulestore.django import modulestore, SignalHandler from contentstore.courseware_index import CoursewareSearchIndexer, LibrarySearchIndexer @@ -17,17 +17,6 @@ from util.module_utils import yield_dynamic_descriptor_descendants log = logging.getLogger(__name__) -# Signal that indicates that a course grading policy has been updated. -# This signal is generated when a grading policy change occurs within -# modulestore for either course or subsection changes. -GRADING_POLICY_CHANGED = Signal( - providing_args=[ - 'user_id', # Integer User ID - 'course_id', # Unicode string representing the course - ] -) - - @receiver(SignalHandler.course_published) def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument """ @@ -50,10 +39,9 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Finally call into the course search subsystem # to kick off an indexing action - if CoursewareSearchIndexer.indexing_is_enabled(): # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from .tasks import update_search_index + from contentstore.tasks import update_search_index update_search_index.delay(unicode(course_key), datetime.now(UTC).isoformat()) @@ -66,7 +54,7 @@ def listen_for_library_update(sender, library_key, **kwargs): # pylint: disable if LibrarySearchIndexer.indexing_is_enabled(): # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from .tasks import update_library_index + from contentstore.tasks import update_library_index update_library_index.delay(unicode(library_key), datetime.now(UTC).isoformat()) diff --git a/cms/djangoapps/contentstore/signals/signals.py b/cms/djangoapps/contentstore/signals/signals.py new file mode 100644 index 0000000000..87f6994165 --- /dev/null +++ b/cms/djangoapps/contentstore/signals/signals.py @@ -0,0 +1,14 @@ +""" +Contentstore signals +""" +from django.dispatch import Signal + +# Signal that indicates that a course grading policy has been updated. +# This signal is generated when a grading policy change occurs within +# modulestore for either course or subsection changes. +GRADING_POLICY_CHANGED = Signal( + providing_args=[ + 'user_id', # Integer User ID + 'course_id', # Unicode string representing the course + ] +) diff --git a/cms/djangoapps/contentstore/startup.py b/cms/djangoapps/contentstore/startup.py deleted file mode 100644 index a19773bb37..0000000000 --- a/cms/djangoapps/contentstore/startup.py +++ /dev/null @@ -1,2 +0,0 @@ -""" will register signal handlers, moved out of __init__.py to ensure correct loading order post Django 1.7 """ -from . import signals # pylint: disable=unused-import diff --git a/cms/djangoapps/contentstore/tests/test_courseware_index.py b/cms/djangoapps/contentstore/tests/test_courseware_index.py index e874a4e0e2..aaa305f0c3 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -44,7 +44,7 @@ from contentstore.courseware_index import ( SearchIndexingError, CourseAboutSearchIndexer, ) -from contentstore.signals import listen_for_course_publish, listen_for_library_update +from contentstore.signals.handlers import listen_for_course_publish, listen_for_library_update from contentstore.utils import reverse_course_url, reverse_usage_url from contentstore.tests.utils import CourseTestCase diff --git a/cms/djangoapps/contentstore/tests/test_gating.py b/cms/djangoapps/contentstore/tests/test_gating.py index 6c46e092d0..202798e8ca 100644 --- a/cms/djangoapps/contentstore/tests/test_gating.py +++ b/cms/djangoapps/contentstore/tests/test_gating.py @@ -1,7 +1,7 @@ """ Unit tests for the gating feature in Studio """ -from contentstore.signals import handle_item_deleted +from contentstore.signals.handlers import handle_item_deleted from milestones.tests.utils import MilestonesTestCaseMixin from mock import patch from openedx.core.lib.gating import api as gating_api @@ -42,16 +42,16 @@ class TestHandleItemDeleted(ModuleStoreTestCase, MilestonesTestCaseMixin): gating_api.add_prerequisite(self.course.id, self.open_seq.location) gating_api.set_required_content(self.course.id, self.gated_seq.location, self.open_seq.location, 100) - @patch('contentstore.signals.gating_api.set_required_content') - @patch('contentstore.signals.gating_api.remove_prerequisite') + @patch('contentstore.signals.handlers.gating_api.set_required_content') + @patch('contentstore.signals.handlers.gating_api.remove_prerequisite') def test_chapter_deleted(self, mock_remove_prereq, mock_set_required): """ Test gating milestone data is cleanup up when course content item is deleted """ handle_item_deleted(usage_key=self.chapter.location, user_id=0) mock_remove_prereq.assert_called_with(self.open_seq.location) mock_set_required.assert_called_with(self.open_seq.location.course_key, self.open_seq.location, None, None) - @patch('contentstore.signals.gating_api.set_required_content') - @patch('contentstore.signals.gating_api.remove_prerequisite') + @patch('contentstore.signals.handlers.gating_api.set_required_content') + @patch('contentstore.signals.handlers.gating_api.remove_prerequisite') def test_sequential_deleted(self, mock_remove_prereq, mock_set_required): """ Test gating milestone data is cleanup up when course content item is deleted """ handle_item_deleted(usage_key=self.open_seq.location, user_id=0) diff --git a/cms/djangoapps/contentstore/tests/test_proctoring.py b/cms/djangoapps/contentstore/tests/test_proctoring.py index 5be8af4928..b0514b2cea 100644 --- a/cms/djangoapps/contentstore/tests/test_proctoring.py +++ b/cms/djangoapps/contentstore/tests/test_proctoring.py @@ -9,7 +9,7 @@ from pytz import UTC from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from contentstore.signals import listen_for_course_publish +from contentstore.signals.handlers import listen_for_course_publish from edx_proctoring.api import ( get_all_exams_for_course, diff --git a/cms/djangoapps/contentstore/views/tests/test_gating.py b/cms/djangoapps/contentstore/views/tests/test_gating.py index 88f5974c53..013271c8c1 100644 --- a/cms/djangoapps/contentstore/views/tests/test_gating.py +++ b/cms/djangoapps/contentstore/views/tests/test_gating.py @@ -128,8 +128,8 @@ class TestSubsectionGating(CourseTestCase): self.assertEqual(resp['prereq_min_score'], 100) self.assertEqual(resp['visibility_state'], VisibilityState.gated) - @patch('contentstore.signals.gating_api.set_required_content') - @patch('contentstore.signals.gating_api.remove_prerequisite') + @patch('contentstore.signals.handlers.gating_api.set_required_content') + @patch('contentstore.signals.handlers.gating_api.remove_prerequisite') def test_delete_item_signal_handler_called(self, mock_remove_prereq, mock_set_required): seq3 = ItemFactory.create( parent_location=self.chapter.location, diff --git a/cms/envs/common.py b/cms/envs/common.py index edf7765be9..728c688fe6 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -868,7 +868,8 @@ INSTALLED_APPS = ( 'django_nose', # For CMS - 'contentstore', + 'contentstore.apps.ContentstoreConfig', + 'openedx.core.djangoapps.contentserver', 'course_creators', 'openedx.core.djangoapps.external_auth',