From 440773a72d7c78d7d10f1781d1aa7b435b0dfb58 Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Mon, 30 Nov 2020 23:34:07 -0800 Subject: [PATCH 1/4] Adding signals and receivers for assessment level reporting to integrated channels --- .../grades/subsection_grade_factory.py | 8 ++++++++ openedx/core/djangoapps/signals/signals.py | 9 +++++++++ .../features/enterprise_support/signals.py | 20 +++++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index 35b6047c2f..8947656715 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -9,6 +9,7 @@ from logging import getLogger from lazy import lazy from submissions import api as submissions_api +from openedx.core.djangoapps.signals.signals import COURSE_ASSESSMENT_GRADE_CHANGED from lms.djangoapps.courseware.model_data import ScoresClient from lms.djangoapps.grades.config import assume_zero_if_absent, should_persist_grades from lms.djangoapps.grades.models import PersistentSubsectionGrade @@ -104,6 +105,13 @@ class SubsectionGradeFactory(object): ) self._update_saved_subsection_grade(subsection.location, grade_model) + COURSE_ASSESSMENT_GRADE_CHANGED.send_robust( + sender=None, + user=self.student, + subsection_id=calculated_grade.location, + subsection_grade=calculated_grade.graded_total.earned + ) + return calculated_grade @lazy diff --git a/openedx/core/djangoapps/signals/signals.py b/openedx/core/djangoapps/signals/signals.py index b0fd6cafa0..64dae48bcb 100644 --- a/openedx/core/djangoapps/signals/signals.py +++ b/openedx/core/djangoapps/signals/signals.py @@ -17,6 +17,15 @@ COURSE_CERT_REVOKED = Signal(providing_args=["user", "course_key", "mode", "stat COURSE_CERT_DATE_CHANGE = Signal(providing_args=["course_key"]) +COURSE_ASSESSMENT_GRADE_CHANGED = Signal( + providing_args=[ + 'user', + 'course_id', + 'subsection_id', + 'subsection_grade', + ] +) + # Signal that indicates that a user has passed a course. COURSE_GRADE_NOW_PASSED = Signal( providing_args=[ diff --git a/openedx/features/enterprise_support/signals.py b/openedx/features/enterprise_support/signals.py index 7d7f472aea..d120a55961 100644 --- a/openedx/features/enterprise_support/signals.py +++ b/openedx/features/enterprise_support/signals.py @@ -11,12 +11,12 @@ from django.contrib.auth.models import User from django.db.models.signals import post_save, pre_save from django.dispatch import receiver from enterprise.models import EnterpriseCourseEnrollment, EnterpriseCustomer, EnterpriseCustomerUser -from integrated_channels.integrated_channel.tasks import transmit_single_learner_data +from integrated_channels.integrated_channel.tasks import transmit_single_learner_data, transmit_subsection_learner_data from slumber.exceptions import HttpClientError from lms.djangoapps.email_marketing.tasks import update_user from openedx.core.djangoapps.commerce.utils import ecommerce_api_client -from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED +from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED, COURSE_ASSESSMENT_GRADE_CHANGED from openedx.features.enterprise_support.api import enterprise_enabled from openedx.features.enterprise_support.tasks import clear_enterprise_customer_data_consent_share_cache from openedx.features.enterprise_support.utils import clear_data_consent_share_cache, is_enterprise_learner @@ -86,6 +86,22 @@ def handle_enterprise_learner_passing_grade(sender, user, course_id, **kwargs): transmit_single_learner_data.apply_async(kwargs=kwargs) +@receiver(COURSE_ASSESSMENT_GRADE_CHANGED) +def handle_enterprise_learner_subsection(sender, user, course_id, subsection_id, subsection_grade, **kwargs): # pylint: disable=unused-argument + """ + Listen for an enterprise learner completing a subsection, transmit data to relevant integrated channel. + """ + if enterprise_enabled() and is_enterprise_learner(user): + kwargs = { + 'username': str(user.username), + 'course_run_id': str(course_id), + 'subsection_id': str(subsection_id), + 'grade': str(subsection_grade), + } + + transmit_subsection_learner_data.apply_async(kwargs=kwargs) + + @receiver(UNENROLL_DONE) def refund_order_voucher(sender, course_enrollment, skip_refund=False, **kwargs): # pylint: disable=unused-argument """ From 66e0f6bf5df13d9b857faa39868e144ece141fe6 Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Mon, 7 Dec 2020 13:23:40 -0800 Subject: [PATCH 2/4] caching enterprise learner lookup --- openedx/features/enterprise_support/utils.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/openedx/features/enterprise_support/utils.py b/openedx/features/enterprise_support/utils.py index 37b026e832..a8e4463aa4 100644 --- a/openedx/features/enterprise_support/utils.py +++ b/openedx/features/enterprise_support/utils.py @@ -7,6 +7,7 @@ import json from crum import get_current_request from django.conf import settings +from django.core.cache import cache from django.urls import NoReverseMatch, reverse from django.utils.translation import ugettext as _ from edx_django_utils.cache import TieredCache, get_cache_key @@ -401,7 +402,15 @@ def is_enterprise_learner(user): Returns: (bool): True if given user is an enterprise learner. """ - return EnterpriseCustomerUser.objects.filter(user_id=user.id).exists() + cached_is_enterprise_key = '{}:user_is_enterprise'.format(user.id) + if cache.get(cached_is_enterprise_key): + return True + + if EnterpriseCustomerUser.objects.filter(user_id=user.id).exists(): + # Cache the enterprise user for one hour. + cache.set(cached_is_enterprise_key, True, expiry=3600) + return True + return False def get_enterprise_slug_login_url(): From 6ef93078f91145f6c032777cd3893ab29716cbc4 Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Tue, 8 Dec 2020 14:40:30 -0800 Subject: [PATCH 3/4] adding unit tests for assessment level reporting and hashing cache key. --- .../grades/subsection_grade_factory.py | 2 +- .../tests/test_subsection_grade_factory.py | 6 ++- .../enterprise_support/tests/test_signals.py | 38 ++++++++++++++++++- .../enterprise_support/tests/test_utils.py | 16 ++++++-- openedx/features/enterprise_support/utils.py | 13 +++++-- 5 files changed, 66 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index 8947656715..e4b07ca50e 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -106,7 +106,7 @@ class SubsectionGradeFactory(object): self._update_saved_subsection_grade(subsection.location, grade_model) COURSE_ASSESSMENT_GRADE_CHANGED.send_robust( - sender=None, + sender=self, user=self.student, subsection_id=calculated_grade.location, subsection_grade=calculated_grade.graded_total.earned diff --git a/lms/djangoapps/grades/tests/test_subsection_grade_factory.py b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py index c3390b716a..91c4f5d8e9 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py @@ -55,8 +55,12 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): test that the score is calculated properly. """ with mock_get_score(1, 2): - grade = self.subsection_grade_factory.update(self.sequence) + with patch( + 'openedx.core.djangoapps.signals.signals.COURSE_ASSESSMENT_GRADE_CHANGED.send_robust' + ) as mock_update_grades_signal: + grade = self.subsection_grade_factory.update(self.sequence) self.assert_grade(grade, 1, 2) + assert mock_update_grades_signal.called def test_write_only_if_engaged(self): """ diff --git a/openedx/features/enterprise_support/tests/test_signals.py b/openedx/features/enterprise_support/tests/test_signals.py index b45f4d5c68..f7ce17ed58 100644 --- a/openedx/features/enterprise_support/tests/test_signals.py +++ b/openedx/features/enterprise_support/tests/test_signals.py @@ -17,7 +17,7 @@ from common.djangoapps.course_modes.tests.factories import CourseModeFactory from lms.djangoapps.certificates.signals import listen_for_passing_grade from openedx.core.djangoapps.commerce.utils import ECOMMERCE_DATE_FORMAT from openedx.core.djangoapps.credit.tests.test_api import TEST_ECOMMERCE_WORKER -from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED +from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED, COURSE_ASSESSMENT_GRADE_CHANGED from openedx.features.enterprise_support.tests import FEATURES_WITH_ENTERPRISE_ENABLED from openedx.features.enterprise_support.tests.factories import ( EnterpriseCourseEnrollmentFactory, @@ -207,3 +207,39 @@ class EnterpriseSupportSignals(SharedModuleStoreTestCase): COURSE_GRADE_NOW_PASSED.send(sender=None, user=self.user, course_id=course_key) mock_task_apply.assert_called_once_with(kwargs=task_kwargs) COURSE_GRADE_NOW_PASSED.connect(listen_for_passing_grade, dispatch_uid='new_passing_learner') + + def test_handle_enterprise_learner_subsection(self): + """ + Test to assert transmit_subsection_learner_data is called when COURSE_ASSESSMENT_GRADE_CHANGED signal is fired. + """ + with patch( + 'integrated_channels.integrated_channel.tasks.transmit_subsection_learner_data.apply_async', + return_value=None + ) as mock_task_apply: + course_key = CourseKey.from_string(self.course_id) + COURSE_ASSESSMENT_GRADE_CHANGED.disconnect() + COURSE_ASSESSMENT_GRADE_CHANGED.send( + sender=None, + user=self.user, + course_id=course_key, + subsection_id='subsection_id', + subsection_grade=1.0 + ) + self.assertFalse(mock_task_apply.called) + + self._create_enterprise_enrollment(self.user.id, self.course_id) + task_kwargs = { + 'username': self.user.username, + 'course_run_id': self.course_id, + 'subsection_id': 'subsection_id', + 'grade': '1.0' + } + COURSE_ASSESSMENT_GRADE_CHANGED.send( + sender=None, + user=self.user, + course_id=course_key, + subsection_id='subsection_id', + subsection_grade=1.0 + ) + mock_task_apply.assert_called_once_with(kwargs=task_kwargs) + COURSE_ASSESSMENT_GRADE_CHANGED.connect(listen_for_passing_grade) diff --git a/openedx/features/enterprise_support/tests/test_utils.py b/openedx/features/enterprise_support/tests/test_utils.py index 74884958f3..c5e9584910 100644 --- a/openedx/features/enterprise_support/tests/test_utils.py +++ b/openedx/features/enterprise_support/tests/test_utils.py @@ -450,11 +450,21 @@ class TestEnterpriseUtils(TestCase): assert '' == generic_name def test_is_enterprise_learner(self): - EnterpriseCustomerUserFactory.create(active=True, user_id=self.user.id) - self.assertTrue(is_enterprise_learner(self.user)) + with mock.patch( + 'django.core.cache.cache.set' + ) as mock_cache_set: + EnterpriseCustomerUserFactory.create(active=True, user_id=self.user.id) + self.assertTrue(is_enterprise_learner(self.user)) + + self.assertTrue(mock_cache_set.called) def test_is_enterprise_learner_no_enterprise_user(self): - self.assertFalse(is_enterprise_learner(self.user)) + with mock.patch( + 'django.core.cache.cache.set' + ) as mock_cache_set: + self.assertFalse(is_enterprise_learner(self.user)) + + self.assertFalse(mock_cache_set.called) @mock.patch('openedx.features.enterprise_support.utils.reverse') def test_get_enterprise_slug_login_url_no_reverse_match(self, mock_reverse): diff --git a/openedx/features/enterprise_support/utils.py b/openedx/features/enterprise_support/utils.py index a8e4463aa4..78e8243c56 100644 --- a/openedx/features/enterprise_support/utils.py +++ b/openedx/features/enterprise_support/utils.py @@ -34,6 +34,13 @@ def get_data_consent_share_cache_key(user_id, course_id): return get_cache_key(type='data_sharing_consent_needed', user_id=user_id, course_id=course_id) +def get_is_enterprise_cache_key(user_id): + """ + Returns cache key for the enterprise learner validation method needed against user_id. + """ + return get_cache_key(type='is_enterprise_learner', user_id=user_id) + + def clear_data_consent_share_cache(user_id, course_id): """ clears data_sharing_consent_needed cache @@ -394,7 +401,7 @@ def get_enterprise_learner_generic_name(request): def is_enterprise_learner(user): """ - Check if the given user belongs to an enterprise. + Check if the given user belongs to an enterprise. Cache the value if an enterprise learner is found. Arguments: user (User): Django User object. @@ -402,13 +409,13 @@ def is_enterprise_learner(user): Returns: (bool): True if given user is an enterprise learner. """ - cached_is_enterprise_key = '{}:user_is_enterprise'.format(user.id) + cached_is_enterprise_key = get_is_enterprise_cache_key(user.id) if cache.get(cached_is_enterprise_key): return True if EnterpriseCustomerUser.objects.filter(user_id=user.id).exists(): # Cache the enterprise user for one hour. - cache.set(cached_is_enterprise_key, True, expiry=3600) + cache.set(cached_is_enterprise_key, True, 3600) return True return False From 2220a1bcb209d288268856d1f8ef062f933b870f Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Wed, 9 Dec 2020 12:30:44 -0800 Subject: [PATCH 4/4] Adding feature flag for subsection grade update signal --- lms/djangoapps/grades/subsection_grade_factory.py | 14 ++++++++------ .../grades/tests/test_subsection_grade_factory.py | 1 + lms/envs/common.py | 12 ++++++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index e4b07ca50e..70339d7293 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -6,6 +6,7 @@ SubsectionGrade Factory Class from collections import OrderedDict from logging import getLogger +from django.conf import settings from lazy import lazy from submissions import api as submissions_api @@ -105,12 +106,13 @@ class SubsectionGradeFactory(object): ) self._update_saved_subsection_grade(subsection.location, grade_model) - COURSE_ASSESSMENT_GRADE_CHANGED.send_robust( - sender=self, - user=self.student, - subsection_id=calculated_grade.location, - subsection_grade=calculated_grade.graded_total.earned - ) + if settings.FEATURES.get('ENABLE_COURSE_ASSESSMENT_GRADE_CHANGE_SIGNAL'): + COURSE_ASSESSMENT_GRADE_CHANGED.send_robust( + sender=self, + user=self.student, + subsection_id=calculated_grade.location, + subsection_grade=calculated_grade.graded_total.earned + ) return calculated_grade diff --git a/lms/djangoapps/grades/tests/test_subsection_grade_factory.py b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py index 91c4f5d8e9..9079ba07a8 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py @@ -49,6 +49,7 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): self.assertIsInstance(grade, ZeroSubsectionGrade) self.assert_grade(grade, 0.0, 1.0) + @patch.dict(settings.FEATURES, {'ENABLE_COURSE_ASSESSMENT_GRADE_CHANGE_SIGNAL': True}) def test_update(self): """ Assuming the underlying score reporting methods work, diff --git a/lms/envs/common.py b/lms/envs/common.py index e4f87818a4..08095a8269 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -692,6 +692,18 @@ FEATURES = { # .. toggle_tickets: https://openedx.atlassian.net/browse/TNL-7273 # .. toggle_warnings: This temporary feature toggle does not have a target removal date. 'ENABLE_ORA_USERNAMES_ON_DATA_EXPORT': False, + + # .. toggle_name: ENABLE_COURSE_ASSESSMENT_GRADE_CHANGE_SIGNAL + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Set to True to start sending signals for assessment level grade updates. Notably, the only + # handler of this signal at the time of this writing sends assessment updates to enterprise integrated channels. + # .. toggle_use_cases: temporary + # .. toggle_creation_date: 2020-12-09 + # .. toggle_target_removal_date: 2021-02-01 + # .. toggle_tickets: https://openedx.atlassian.net/browse/ENT-3818 + # .. toggle_warnings: None. + 'ENABLE_COURSE_ASSESSMENT_GRADE_CHANGE_SIGNAL': False, } # Specifies extra XBlock fields that should available when requested via the Course Blocks API