Site config flag for Learner Records
Allow site admins to disable integration for the Learner Records feature in Credentials. LEARNER-5987
This commit is contained in:
committed by
Michael Terry
parent
a70a62feda
commit
3222d3fd13
@@ -1440,7 +1440,7 @@ class ProgressPageTests(ProgressPageBaseTests):
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
|
||||
@ddt.data(
|
||||
(False, 45, 29),
|
||||
(False, 45, 28),
|
||||
(True, 37, 24)
|
||||
)
|
||||
@ddt.unpack
|
||||
|
||||
@@ -104,21 +104,21 @@ class TestCourseGradeFactory(GradeTestBase):
|
||||
with self.assertNumQueries(2):
|
||||
_assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5
|
||||
|
||||
num_queries = 7
|
||||
num_queries = 6
|
||||
with self.assertNumQueries(num_queries), mock_get_score(1, 4):
|
||||
grade_factory.update(self.request.user, self.course, force_update_subsections=False)
|
||||
|
||||
with self.assertNumQueries(2):
|
||||
_assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25
|
||||
|
||||
num_queries = 21
|
||||
num_queries = 20
|
||||
with self.assertNumQueries(num_queries), mock_get_score(2, 2):
|
||||
grade_factory.update(self.request.user, self.course, force_update_subsections=True)
|
||||
|
||||
with self.assertNumQueries(2):
|
||||
_assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0
|
||||
|
||||
num_queries = 24
|
||||
num_queries = 23
|
||||
with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero
|
||||
grade_factory.update(self.request.user, self.course, force_update_subsections=True)
|
||||
|
||||
|
||||
15
openedx/core/djangoapps/credentials/docs/site_config.rst
Normal file
15
openedx/core/djangoapps/credentials/docs/site_config.rst
Normal file
@@ -0,0 +1,15 @@
|
||||
Site Configuration
|
||||
==================
|
||||
|
||||
These are the site configuration flags that affect Credentials features.
|
||||
|
||||
ENABLE_CREDENTIALS_RECORDS
|
||||
--------------------------
|
||||
|
||||
Controls whether the LMS integrates with the Learner Records feature in Credentials. Specifically, this turns on some web buttons that link to a learner's record on the Credentials IDA and enables some data being passed to Credentials related to those records.
|
||||
|
||||
If you have had this disabled and decide to enable it, you will need to backpopulate the data that was not sent while the feature was disabled. Look into running the ``notify_credentials`` management command.
|
||||
|
||||
Note that Credentials has a similar site config option that you'll want to keep in sync with this setting.
|
||||
|
||||
Default is ``True``.
|
||||
@@ -26,6 +26,7 @@ from lms.djangoapps.certificates.models import GeneratedCertificate
|
||||
from lms.djangoapps.grades.models import PersistentCourseGrade
|
||||
from openedx.core.djangoapps.credentials.signals import handle_cert_change, send_grade_if_interesting
|
||||
from openedx.core.djangoapps.programs.signals import handle_course_cert_changed
|
||||
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
|
||||
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -101,6 +102,11 @@ class Command(BaseCommand):
|
||||
action='store_true',
|
||||
help='Just show a preview of what would happen.',
|
||||
)
|
||||
parser.add_argument(
|
||||
'--site',
|
||||
default=None,
|
||||
help="Site domain to notify for (if not specified, all sites are notified). Uses course_org_filter.",
|
||||
)
|
||||
parser.add_argument(
|
||||
'--courses',
|
||||
nargs='+',
|
||||
@@ -131,14 +137,20 @@ class Command(BaseCommand):
|
||||
|
||||
def handle(self, *args, **options):
|
||||
log.info(
|
||||
"notify_credentials starting, dry-run=%s, delay=%d seconds",
|
||||
"notify_credentials starting, dry-run=%s, site=%s, delay=%d seconds",
|
||||
options['dry_run'],
|
||||
options['site'],
|
||||
options['delay']
|
||||
)
|
||||
|
||||
cert_filter_args = {}
|
||||
grade_filter_args = {}
|
||||
|
||||
try:
|
||||
site_config = SiteConfiguration.objects.get(site__domain=options['site']) if options['site'] else None
|
||||
except SiteConfiguration.DoesNotExist:
|
||||
log.error('No site configuration found for site %s', options['site'])
|
||||
|
||||
if options['courses']:
|
||||
course_keys = self.get_course_keys(options['courses'])
|
||||
cert_filter_args['course_id__in'] = course_keys
|
||||
@@ -162,15 +174,22 @@ class Command(BaseCommand):
|
||||
if options['dry_run']:
|
||||
self.print_dry_run(certs, grades)
|
||||
else:
|
||||
self.send_notifications(certs, grades, delay=options['delay'], page_size=options['page_size'])
|
||||
self.send_notifications(certs, grades,
|
||||
site_config=site_config,
|
||||
delay=options['delay'],
|
||||
page_size=options['page_size'])
|
||||
|
||||
log.info('notify_credentials finished')
|
||||
|
||||
def send_notifications(self, certs, grades, delay=0, page_size=0):
|
||||
def send_notifications(self, certs, grades, site_config=None, delay=0, page_size=0):
|
||||
""" Run actual handler commands for the provided certs and grades. """
|
||||
|
||||
# First, do certs
|
||||
for i, cert in paged_query(certs, delay, page_size):
|
||||
if site_config and not site_config.has_org(cert.course_id.org):
|
||||
log.info("Skipping credential changes %d for certificate %s", i, certstr(cert))
|
||||
continue
|
||||
|
||||
log.info(
|
||||
"Handling credential changes %d for certificate %s",
|
||||
i, certstr(cert),
|
||||
@@ -188,6 +207,10 @@ class Command(BaseCommand):
|
||||
|
||||
# Then do grades
|
||||
for i, grade in paged_query(grades, delay, page_size):
|
||||
if site_config and not site_config.has_org(grade.course_id.org):
|
||||
log.info("Skipping grade changes %d for grade %s", i, gradestr(grade))
|
||||
continue
|
||||
|
||||
log.info(
|
||||
"Handling grade changes %d for grade %s",
|
||||
i, gradestr(grade),
|
||||
|
||||
@@ -14,6 +14,7 @@ from freezegun import freeze_time
|
||||
|
||||
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
|
||||
from lms.djangoapps.grades.models import PersistentCourseGrade
|
||||
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
@@ -36,7 +37,7 @@ class TestNotifyCredentials(TestCase):
|
||||
with freeze_time(datetime(2017, 2, 1)):
|
||||
self.cert2 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+2')
|
||||
with freeze_time(datetime(2017, 3, 1)):
|
||||
self.cert3 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+3')
|
||||
self.cert3 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:testX+Test+3')
|
||||
print('self.cert1.modified_date', self.cert1.modified_date)
|
||||
|
||||
# No factory for these
|
||||
@@ -47,7 +48,7 @@ class TestNotifyCredentials(TestCase):
|
||||
self.grade2 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+2',
|
||||
percent_grade=1)
|
||||
with freeze_time(datetime(2017, 3, 1)):
|
||||
self.grade3 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+3',
|
||||
self.grade3 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:testX+Test+3',
|
||||
percent_grade=1)
|
||||
print('self.grade1.modified', self.grade1.modified)
|
||||
|
||||
@@ -119,3 +120,14 @@ class TestNotifyCredentials(TestCase):
|
||||
reset_queries()
|
||||
call_command(Command(), '--start-date', '2017-01-01', '--page-size=2')
|
||||
self.assertEqual(len(connection.queries), baseline + 2) # one extra page query each for certs & grades
|
||||
|
||||
@mock.patch(COMMAND_MODULE + '.send_grade_if_interesting')
|
||||
@mock.patch(COMMAND_MODULE + '.handle_cert_change')
|
||||
def test_site(self, mock_grade_interesting, mock_cert_change):
|
||||
site_config = SiteConfigurationFactory.create(
|
||||
values={'course_org_filter': ['testX']},
|
||||
)
|
||||
|
||||
call_command(Command(), '--site', site_config.site.domain, '--start-date', '2017-01-01')
|
||||
self.assertEqual(mock_grade_interesting.call_count, 1)
|
||||
self.assertEqual(mock_cert_change.call_count, 1)
|
||||
|
||||
@@ -96,6 +96,9 @@ class CredentialsApiConfig(ConfigurationModel):
|
||||
# Temporarily disable this feature while we work on it
|
||||
if not STUDENT_RECORDS_FLAG.is_enabled():
|
||||
return None
|
||||
# Not every site wants the Learner Records feature, so we allow opting out.
|
||||
if not helpers.get_value('ENABLE_LEARNER_RECORDS', True):
|
||||
return None
|
||||
root = helpers.get_value('CREDENTIALS_PUBLIC_SERVICE_URL', settings.CREDENTIALS_PUBLIC_SERVICE_URL)
|
||||
return urljoin(root, '/records/')
|
||||
|
||||
|
||||
@@ -8,6 +8,8 @@ from django.contrib.sites.models import Site
|
||||
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
|
||||
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
|
||||
from openedx.core.djangoapps.catalog.utils import get_programs
|
||||
from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
|
||||
from openedx.core.djangoapps.site_configuration import helpers
|
||||
|
||||
from .tasks.v1.tasks import send_grade_to_credentials
|
||||
|
||||
@@ -43,6 +45,14 @@ def is_course_run_in_a_program(course_run_key):
|
||||
def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, percent_grade):
|
||||
""" Checks if grade is interesting to Credentials and schedules a Celery task if so. """
|
||||
|
||||
# Avoid scheduling new tasks if certification is disabled. (Grades are a part of the records/cert story)
|
||||
if not CredentialsApiConfig.current().is_learner_issuance_enabled:
|
||||
return
|
||||
|
||||
# Avoid scheduling new tasks if learner records are disabled for this site.
|
||||
if not helpers.get_value_for_org(course_run_key.org, 'ENABLE_LEARNER_RECORDS', True):
|
||||
return
|
||||
|
||||
# Grab mode/status if we don't have them in hand
|
||||
if mode is None or status is None:
|
||||
try:
|
||||
|
||||
@@ -10,7 +10,7 @@ from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
|
||||
from lms.djangoapps.grades.tests.utils import mock_passing_grade
|
||||
from openedx.core.djangoapps.catalog.tests.factories import CourseFactory, CourseRunFactory, ProgramFactory
|
||||
from openedx.core.djangoapps.credentials.signals import is_course_run_in_a_program, send_grade_if_interesting
|
||||
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
|
||||
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
from student.tests.factories import UserFactory
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
@@ -22,6 +22,11 @@ SIGNALS_MODULE = 'openedx.core.djangoapps.credentials.signals'
|
||||
|
||||
@ddt.ddt
|
||||
@skip_unless_lms
|
||||
@mock.patch(
|
||||
'openedx.core.djangoapps.credentials.models.CredentialsApiConfig.is_learner_issuance_enabled',
|
||||
new_callable=mock.PropertyMock,
|
||||
return_value=True,
|
||||
)
|
||||
@mock.patch(SIGNALS_MODULE + '.send_grade_to_credentials')
|
||||
@mock.patch(SIGNALS_MODULE + '.is_course_run_in_a_program')
|
||||
class TestCredentialsSignalsSendGrade(TestCase):
|
||||
@@ -42,7 +47,7 @@ class TestCredentialsSignalsSendGrade(TestCase):
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_send_grade_if_right_cert(self, called, mode, status, mock_is_course_run_in_a_program,
|
||||
mock_send_grade_to_credentials):
|
||||
mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled):
|
||||
mock_is_course_run_in_a_program.return_value = True
|
||||
|
||||
# Test direct send
|
||||
@@ -60,19 +65,20 @@ class TestCredentialsSignalsSendGrade(TestCase):
|
||||
send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0)
|
||||
self.assertIs(mock_send_grade_to_credentials.delay.called, called)
|
||||
|
||||
def test_send_grade_missing_cert(self, _, mock_send_grade_to_credentials):
|
||||
def test_send_grade_missing_cert(self, _, mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled):
|
||||
send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0)
|
||||
self.assertFalse(mock_send_grade_to_credentials.delay.called)
|
||||
|
||||
@ddt.data([True], [False])
|
||||
@ddt.unpack
|
||||
def test_send_grade_if_in_a_program(self, in_program, mock_is_course_run_in_a_program,
|
||||
mock_send_grade_to_credentials):
|
||||
mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled):
|
||||
mock_is_course_run_in_a_program.return_value = in_program
|
||||
send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', 'A', 1.0)
|
||||
self.assertIs(mock_send_grade_to_credentials.delay.called, in_program)
|
||||
|
||||
def test_send_grade_queries_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials):
|
||||
def test_send_grade_queries_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials,
|
||||
_mock_is_learner_issuance_enabled):
|
||||
mock_is_course_run_in_a_program.return_value = True
|
||||
|
||||
with mock_passing_grade('B', 0.81):
|
||||
@@ -83,11 +89,36 @@ class TestCredentialsSignalsSendGrade(TestCase):
|
||||
mock_send_grade_to_credentials.delay.reset_mock()
|
||||
|
||||
@mock.patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
|
||||
def test_send_grade_without_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials):
|
||||
def test_send_grade_without_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials,
|
||||
_mock_is_learner_issuance_enabled):
|
||||
mock_is_course_run_in_a_program.return_value = True
|
||||
send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None)
|
||||
self.assertFalse(mock_send_grade_to_credentials.delay.called)
|
||||
|
||||
def test_send_grade_without_issuance_enabled(self, _mock_is_course_run_in_a_program,
|
||||
mock_send_grade_to_credentials, mock_is_learner_issuance_enabled):
|
||||
mock_is_learner_issuance_enabled.return_value = False
|
||||
send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None)
|
||||
self.assertTrue(mock_is_learner_issuance_enabled.called)
|
||||
self.assertFalse(mock_send_grade_to_credentials.delay.called)
|
||||
|
||||
def test_send_grade_records_enabled(self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials,
|
||||
_mock_is_learner_issuance_enabled):
|
||||
site_config = SiteConfigurationFactory.create(
|
||||
values={'course_org_filter': [self.key.org]},
|
||||
)
|
||||
|
||||
# Correctly sent
|
||||
send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None)
|
||||
self.assertTrue(mock_send_grade_to_credentials.delay.called)
|
||||
mock_send_grade_to_credentials.delay.reset_mock()
|
||||
|
||||
# Correctly not sent
|
||||
site_config.values['ENABLE_LEARNER_RECORDS'] = False
|
||||
site_config.save()
|
||||
send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None)
|
||||
self.assertFalse(mock_send_grade_to_credentials.delay.called)
|
||||
|
||||
|
||||
@skip_unless_lms
|
||||
@mock.patch(SIGNALS_MODULE + '.get_programs')
|
||||
|
||||
@@ -6,6 +6,7 @@ import logging
|
||||
from django.dispatch import receiver
|
||||
|
||||
from openedx.core.djangoapps.signals.signals import COURSE_CERT_AWARDED, COURSE_CERT_CHANGED
|
||||
from openedx.core.djangoapps.site_configuration import helpers
|
||||
|
||||
LOGGER = logging.getLogger(__name__)
|
||||
|
||||
@@ -84,6 +85,11 @@ def handle_course_cert_changed(sender, user, course_key, mode, status, **kwargs)
|
||||
if not CredentialsApiConfig.current().is_learner_issuance_enabled:
|
||||
return
|
||||
|
||||
# Avoid scheduling new tasks if learner records are disabled for this site (right now, course certs are only
|
||||
# used for learner records -- when that changes, we can remove this bit and always send course certs).
|
||||
if not helpers.get_value_for_org(course_key.org, 'ENABLE_LEARNER_RECORDS', True):
|
||||
return
|
||||
|
||||
# schedule background task to process
|
||||
LOGGER.debug(
|
||||
'handling COURSE_CERT_CHANGED: username=%s, course_key=%s, mode=%s, status=%s',
|
||||
|
||||
@@ -6,14 +6,16 @@ from django.test import TestCase
|
||||
from nose.plugins.attrib import attr
|
||||
import mock
|
||||
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
from openedx.core.djangoapps.signals.signals import COURSE_CERT_AWARDED, COURSE_CERT_CHANGED
|
||||
from openedx.core.djangoapps.programs.signals import handle_course_cert_awarded, handle_course_cert_changed
|
||||
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
|
||||
TEST_USERNAME = 'test-user'
|
||||
TEST_COURSE_KEY = 'test-course'
|
||||
TEST_COURSE_KEY = CourseKey.from_string('course-v1:edX+test_course+1')
|
||||
|
||||
|
||||
@attr(shard=2)
|
||||
@@ -92,6 +94,10 @@ class CertChangedReceiverTest(TestCase):
|
||||
Tests for the `handle_course_cert_changed` signal handler function.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super(CertChangedReceiverTest, self).setUp()
|
||||
self.user = UserFactory.create(username=TEST_USERNAME)
|
||||
|
||||
@property
|
||||
def signal_kwargs(self):
|
||||
"""
|
||||
@@ -99,8 +105,8 @@ class CertChangedReceiverTest(TestCase):
|
||||
"""
|
||||
return dict(
|
||||
sender=self.__class__,
|
||||
user=UserFactory.create(username=TEST_USERNAME),
|
||||
course_key='test-course',
|
||||
user=self.user,
|
||||
course_key=TEST_COURSE_KEY,
|
||||
mode='test-mode',
|
||||
status='test-status',
|
||||
)
|
||||
@@ -115,7 +121,7 @@ class CertChangedReceiverTest(TestCase):
|
||||
known to take place inside the function.
|
||||
"""
|
||||
COURSE_CERT_CHANGED.send(**self.signal_kwargs)
|
||||
self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1)
|
||||
self.assertEqual(mock_is_learner_issuance_enabled.call_count, 2)
|
||||
|
||||
def test_credentials_disabled(self, mock_is_learner_issuance_enabled, mock_task):
|
||||
"""
|
||||
@@ -137,4 +143,22 @@ class CertChangedReceiverTest(TestCase):
|
||||
|
||||
self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1)
|
||||
self.assertEqual(mock_task.call_count, 1)
|
||||
self.assertEqual(mock_task.call_args[0], (TEST_USERNAME, TEST_COURSE_KEY))
|
||||
self.assertEqual(mock_task.call_args[0], (TEST_USERNAME, str(TEST_COURSE_KEY)))
|
||||
|
||||
def test_records_enabled(self, mock_is_learner_issuance_enabled, mock_task):
|
||||
mock_is_learner_issuance_enabled.return_value = True
|
||||
|
||||
site_config = SiteConfigurationFactory.create(
|
||||
values={'course_org_filter': ['edX']},
|
||||
)
|
||||
|
||||
# Correctly sent
|
||||
handle_course_cert_changed(**self.signal_kwargs)
|
||||
self.assertTrue(mock_task.called)
|
||||
mock_task.reset_mock()
|
||||
|
||||
# Correctly not sent
|
||||
site_config.values['ENABLE_LEARNER_RECORDS'] = False
|
||||
site_config.save()
|
||||
handle_course_cert_changed(**self.signal_kwargs)
|
||||
self.assertFalse(mock_task.called)
|
||||
|
||||
Reference in New Issue
Block a user