From b02c744790f6506b2dfbe46c7d2bd6b68cf7534b Mon Sep 17 00:00:00 2001 From: John Eskew Date: Thu, 2 Nov 2017 11:53:17 -0400 Subject: [PATCH 1/4] Move lti_provider startup.py to AppConfig::ready. --- lms/djangoapps/lti_provider/apps.py | 16 ++++++++++++++++ lms/djangoapps/lti_provider/startup.py | 4 ---- lms/envs/aws.py | 2 +- lms/envs/test.py | 2 +- lms/envs/yaml_config.py | 2 +- 5 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 lms/djangoapps/lti_provider/apps.py delete mode 100644 lms/djangoapps/lti_provider/startup.py diff --git a/lms/djangoapps/lti_provider/apps.py b/lms/djangoapps/lti_provider/apps.py new file mode 100644 index 0000000000..a9d94ae21c --- /dev/null +++ b/lms/djangoapps/lti_provider/apps.py @@ -0,0 +1,16 @@ +""" +Configuration for the lti_provider Django application. +""" +from django.apps import AppConfig + + +class LtiProviderConfig(AppConfig): + """ + Configuration class for the lti_provider Django application. + """ + name = 'lti_provider' + verbose_name = "LTI Provider" + + def ready(self): + # Import the tasks module to ensure that signal handlers are registered. + from . import tasks # pylint: disable=unused-import diff --git a/lms/djangoapps/lti_provider/startup.py b/lms/djangoapps/lti_provider/startup.py deleted file mode 100644 index 24f5f6e505..0000000000 --- a/lms/djangoapps/lti_provider/startup.py +++ /dev/null @@ -1,4 +0,0 @@ -"""Code run at server start up to initialize the lti_provider app.""" - -# Import the tasks module to ensure that signal handlers are registered. -import lms.djangoapps.lti_provider.tasks # pylint: disable=unused-import diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 7be1753a7c..68d4f420ca 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -869,7 +869,7 @@ CREDIT_PROVIDER_SECRET_KEYS = AUTH_TOKENS.get("CREDIT_PROVIDER_SECRET_KEYS", {}) ##################### LTI Provider ##################### if FEATURES.get('ENABLE_LTI_PROVIDER'): - INSTALLED_APPS.append('lti_provider') + INSTALLED_APPS.append('lti_provider.apps.LtiProviderConfig') AUTHENTICATION_BACKENDS.append('lti_provider.users.LtiBackend') LTI_USER_EMAIL_DOMAIN = ENV_TOKENS.get('LTI_USER_EMAIL_DOMAIN', 'lti.example.com') diff --git a/lms/envs/test.py b/lms/envs/test.py index d683b05c47..41253eddfd 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -571,7 +571,7 @@ PROFILE_IMAGE_MIN_BYTES = 100 # Enable the LTI provider feature for testing FEATURES['ENABLE_LTI_PROVIDER'] = True -INSTALLED_APPS.append('lti_provider') +INSTALLED_APPS.append('lti_provider.apps.LtiProviderConfig') AUTHENTICATION_BACKENDS.append('lti_provider.users.LtiBackend') # ORGANIZATIONS diff --git a/lms/envs/yaml_config.py b/lms/envs/yaml_config.py index 83ed8b56b7..9517144ba5 100644 --- a/lms/envs/yaml_config.py +++ b/lms/envs/yaml_config.py @@ -319,7 +319,7 @@ if FEATURES.get('INDIVIDUAL_DUE_DATES'): ##################### LTI Provider ##################### if FEATURES.get('ENABLE_LTI_PROVIDER'): - INSTALLED_APPS.append('lti_provider') + INSTALLED_APPS.append('lti_provider.apps.LtiProviderConfig') AUTHENTICATION_BACKENDS.append('lti_provider.users.LtiBackend') ################################ Settings for Credentials Service ################################ From 7b4a3c1714e5439e1adaf1fcbb20748f65a4c1d0 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Thu, 2 Nov 2017 12:54:27 -0400 Subject: [PATCH 2/4] Move email_marketing startup.py to AppConfig::ready(). Remove query counting of LMS signals. --- lms/djangoapps/email_marketing/apps.py | 16 ++++++++++++++++ lms/djangoapps/email_marketing/startup.py | 3 --- lms/envs/common.py | 2 +- .../lang_pref/tests/test_middleware.py | 18 ++---------------- 4 files changed, 19 insertions(+), 20 deletions(-) create mode 100644 lms/djangoapps/email_marketing/apps.py delete mode 100644 lms/djangoapps/email_marketing/startup.py diff --git a/lms/djangoapps/email_marketing/apps.py b/lms/djangoapps/email_marketing/apps.py new file mode 100644 index 0000000000..090467fff1 --- /dev/null +++ b/lms/djangoapps/email_marketing/apps.py @@ -0,0 +1,16 @@ +""" +Configuration for the email_marketing Django application. +""" +from django.apps import AppConfig + + +class EmailMarketingConfig(AppConfig): + """ + Configuration class for the email_marketing Django application. + """ + name = 'email_marketing' + verbose_name = "Email Marketing" + + def ready(self): + # Register the signal handlers. + from . import signals # pylint: disable=unused-import diff --git a/lms/djangoapps/email_marketing/startup.py b/lms/djangoapps/email_marketing/startup.py deleted file mode 100644 index 5c0c920fc4..0000000000 --- a/lms/djangoapps/email_marketing/startup.py +++ /dev/null @@ -1,3 +0,0 @@ -""" email_marketing app. """ -# this is here to support registering the signals in signals.py -from email_marketing import signals # pylint: disable=unused-import diff --git a/lms/envs/common.py b/lms/envs/common.py index 378680e1f6..9dc8851b82 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2289,7 +2289,7 @@ INSTALLED_APPS = [ 'django_sites_extensions', # Email marketing integration - 'email_marketing', + 'email_marketing.apps.EmailMarketingConfig', # additional release utilities to ease automation 'release_util', diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index f588c72fbc..385a142416 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -231,14 +231,7 @@ class TestUserPreferenceMiddleware(TestCase): # No preference yet, should write to the database self.assertEqual(get_user_preference(self.user, LANGUAGE_KEY), None) - - # The 'email_marketing' app is installed in the LMS env but not the CMS env. It listens for the - # USER_FIELD_CHANGED signal (utils.model_utils) and does a query to check the EmailMarketingConfiguration - # table to see if Sailthru integreation is enabled. - expected_queries = 6 if 'email_marketing' in settings.INSTALLED_APPS else 5 - with self.assertNumQueries(expected_queries): - self.middleware.process_request(self.request) - + self.middleware.process_request(self.request) self.assertEqual(get_user_preference(self.user, LANGUAGE_KEY), 'es') response = mock.Mock(spec=HttpResponse) @@ -261,14 +254,7 @@ class TestUserPreferenceMiddleware(TestCase): # Cookie changed, should write to the database again self.request.COOKIES[settings.LANGUAGE_COOKIE] = 'en' - - # The 'email_marketing' app is installed in the LMS env but not the CMS env. It listens for the - # USER_FIELD_CHANGED signal (utils.model_utils) and does a query to check the EmailMarketingConfiguration - # table to see if Sailthru integreation is enabled. - expected_queries = 6 if 'email_marketing' in settings.INSTALLED_APPS else 5 - with self.assertNumQueries(expected_queries): - self.middleware.process_request(self.request) - + self.middleware.process_request(self.request) self.assertEqual(get_user_preference(self.user, LANGUAGE_KEY), 'en') with self.assertNumQueries(1): From 6e7cba2803ff012fb043c00addd3651ad53e2758 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Thu, 2 Nov 2017 15:10:46 -0400 Subject: [PATCH 3/4] Add doctrings to module/class. --- common/djangoapps/edxmako/apps.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/edxmako/apps.py b/common/djangoapps/edxmako/apps.py index 3cf4130924..20fe2a5a1c 100644 --- a/common/djangoapps/edxmako/apps.py +++ b/common/djangoapps/edxmako/apps.py @@ -1,10 +1,15 @@ - +""" +Configuration for the edxmako Django application. +""" from django.apps import AppConfig from django.conf import settings from . import add_lookup, clear_lookups class EdxMakoConfig(AppConfig): + """ + Configuration class for the edxmako Django application. + """ name = 'edxmako' verbose_name = "edX Mako Templating" From 878d3eff6edbacc3deb623c2823dfa3d78cde266 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 6 Nov 2017 16:41:05 -0500 Subject: [PATCH 4/4] Split signals.py from tasks.py file. --- lms/djangoapps/lti_provider/apps.py | 2 +- lms/djangoapps/lti_provider/signals.py | 69 ++++++++++++++++++++++++++ lms/djangoapps/lti_provider/tasks.py | 59 +--------------------- 3 files changed, 71 insertions(+), 59 deletions(-) create mode 100644 lms/djangoapps/lti_provider/signals.py diff --git a/lms/djangoapps/lti_provider/apps.py b/lms/djangoapps/lti_provider/apps.py index a9d94ae21c..8d271688af 100644 --- a/lms/djangoapps/lti_provider/apps.py +++ b/lms/djangoapps/lti_provider/apps.py @@ -13,4 +13,4 @@ class LtiProviderConfig(AppConfig): def ready(self): # Import the tasks module to ensure that signal handlers are registered. - from . import tasks # pylint: disable=unused-import + from . import signals # pylint: disable=unused-import diff --git a/lms/djangoapps/lti_provider/signals.py b/lms/djangoapps/lti_provider/signals.py new file mode 100644 index 0000000000..ff7eaceb19 --- /dev/null +++ b/lms/djangoapps/lti_provider/signals.py @@ -0,0 +1,69 @@ +""" +Signals handlers for the lti_provider Django app. +""" +from __future__ import absolute_import +import logging + +from django.conf import settings +from django.dispatch import receiver + +import lti_provider.outcomes as outcomes +from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED +from lti_provider.views import parse_course_and_usage_keys +from xmodule.modulestore.django import modulestore +from .tasks import send_composite_outcome, send_leaf_outcome + +log = logging.getLogger(__name__) + + +def increment_assignment_versions(course_key, usage_key, user_id): + """ + Update the version numbers for all assignments that are affected by a score + change event. Returns a list of all affected assignments. + """ + problem_descriptor = modulestore().get_item(usage_key) + # Get all assignments involving the current problem for which the campus LMS + # is expecting a grade. There may be many possible graded assignments, if + # a problem has been added several times to a course at different + # granularities (such as the unit or the vertical). + assignments = outcomes.get_assignments_for_problem( + problem_descriptor, user_id, course_key + ) + for assignment in assignments: + assignment.version_number += 1 + assignment.save() + return assignments + + +@receiver(PROBLEM_WEIGHTED_SCORE_CHANGED) +def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument + """ + Consume signals that indicate score changes. See the definition of + PROBLEM_WEIGHTED_SCORE_CHANGED for a description of the signal. + """ + points_possible = kwargs.get('weighted_possible', None) + points_earned = kwargs.get('weighted_earned', None) + user_id = kwargs.get('user_id', None) + course_id = kwargs.get('course_id', None) + usage_id = kwargs.get('usage_id', None) + + if None not in (points_earned, points_possible, user_id, course_id): + course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id) + assignments = increment_assignment_versions(course_key, usage_key, user_id) + for assignment in assignments: + if assignment.usage_key == usage_key: + send_leaf_outcome.delay( + assignment.id, points_earned, points_possible + ) + else: + send_composite_outcome.apply_async( + (user_id, course_id, assignment.id, assignment.version_number), + countdown=settings.LTI_AGGREGATE_SCORE_PASSBACK_DELAY + ) + else: + log.error( + "Outcome Service: Required signal parameter is None. " + "points_possible: %s, points_earned: %s, user_id: %s, " + "course_id: %s, usage_id: %s", + points_possible, points_earned, user_id, course_id, usage_id + ) diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index 549c38afb4..6839ef154f 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -4,73 +4,16 @@ Asynchronous tasks for the LTI provider app. import logging -from django.conf import settings from django.contrib.auth.models import User -from django.dispatch import receiver from opaque_keys.edx.keys import CourseKey import lti_provider.outcomes as outcomes from lms import CELERY_APP from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory -from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from lti_provider.models import GradedAssignment -from lti_provider.views import parse_course_and_usage_keys from xmodule.modulestore.django import modulestore -log = logging.getLogger("edx.lti_provider") - - -@receiver(PROBLEM_WEIGHTED_SCORE_CHANGED) -def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument - """ - Consume signals that indicate score changes. See the definition of - PROBLEM_WEIGHTED_SCORE_CHANGED for a description of the signal. - """ - points_possible = kwargs.get('weighted_possible', None) - points_earned = kwargs.get('weighted_earned', None) - user_id = kwargs.get('user_id', None) - course_id = kwargs.get('course_id', None) - usage_id = kwargs.get('usage_id', None) - - if None not in (points_earned, points_possible, user_id, course_id): - course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id) - assignments = increment_assignment_versions(course_key, usage_key, user_id) - for assignment in assignments: - if assignment.usage_key == usage_key: - send_leaf_outcome.delay( - assignment.id, points_earned, points_possible - ) - else: - send_composite_outcome.apply_async( - (user_id, course_id, assignment.id, assignment.version_number), - countdown=settings.LTI_AGGREGATE_SCORE_PASSBACK_DELAY - ) - else: - log.error( - "Outcome Service: Required signal parameter is None. " - "points_possible: %s, points_earned: %s, user_id: %s, " - "course_id: %s, usage_id: %s", - points_possible, points_earned, user_id, course_id, usage_id - ) - - -def increment_assignment_versions(course_key, usage_key, user_id): - """ - Update the version numbers for all assignments that are affected by a score - change event. Returns a list of all affected assignments. - """ - problem_descriptor = modulestore().get_item(usage_key) - # Get all assignments involving the current problem for which the campus LMS - # is expecting a grade. There may be many possible graded assignments, if - # a problem has been added several times to a course at different - # granularities (such as the unit or the vertical). - assignments = outcomes.get_assignments_for_problem( - problem_descriptor, user_id, course_key - ) - for assignment in assignments: - assignment.version_number += 1 - assignment.save() - return assignments +log = logging.getLogger(__name__) @CELERY_APP.task(name='lti_provider.tasks.send_composite_outcome')