From bd038bab3cf02df147e754f7743e46b68b43bac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Thu, 1 Oct 2020 10:34:17 +0200 Subject: [PATCH] Add default value for learner records feature toggle in settings The learner records feature had to be enabled/disabled via site-specific configuration models, which is inconvenient for platforms that want to disable this feature globally. Here, we introduce a ENABLE_LEARNER_RECORDS feature toggle in the lms/cms settings that makes it possible to disable this feature on all sites. Because this feature toggle is set to True by default, this will not modify the behaviour of existing platforms. --- .../core/djangoapps/credentials/helpers.py | 32 +++++++++++++++++++ openedx/core/djangoapps/credentials/models.py | 4 ++- .../core/djangoapps/credentials/signals.py | 4 +-- .../credentials/tests/test_signals.py | 13 +++++++- openedx/core/djangoapps/programs/signals.py | 5 +-- 5 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 openedx/core/djangoapps/credentials/helpers.py diff --git a/openedx/core/djangoapps/credentials/helpers.py b/openedx/core/djangoapps/credentials/helpers.py new file mode 100644 index 0000000000..58b56747ea --- /dev/null +++ b/openedx/core/djangoapps/credentials/helpers.py @@ -0,0 +1,32 @@ +""" +Helpers for the credentials service. +""" + +from edx_toggles.toggles import SettingDictToggle + +from openedx.core.djangoapps.site_configuration import helpers as config_helpers + +# .. toggle_name: FEATURES['ENABLE_LEARNER_RECORDS'] +# .. toggle_implementation: SettingDictToggle +# .. toggle_default: True +# .. toggle_description: Enable learner records for the whole platform. This setting may be overridden by site- and +# org-specific site configurations with the same name. +# .. toggle_warnings: Enabling this feature requires that the definition of the ``CREDENTIALS_PUBLIC_SERVICE_URL`` +# setting. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2020-10-01 +ENABLE_LEARNER_RECORDS = SettingDictToggle( + "FEATURES", "ENABLE_LEARNER_RECORDS", default=True, module_name=__name__ +) + + +def is_learner_records_enabled(): + return config_helpers.get_value( + "ENABLE_LEARNER_RECORDS", ENABLE_LEARNER_RECORDS.is_enabled() + ) + + +def is_learner_records_enabled_for_org(org): + return config_helpers.get_value_for_org( + org, "ENABLE_LEARNER_RECORDS", ENABLE_LEARNER_RECORDS.is_enabled() + ) diff --git a/openedx/core/djangoapps/credentials/models.py b/openedx/core/djangoapps/credentials/models.py index a9568d79b7..ccc0d2e4ab 100644 --- a/openedx/core/djangoapps/credentials/models.py +++ b/openedx/core/djangoapps/credentials/models.py @@ -13,6 +13,8 @@ from six.moves.urllib.parse import urljoin # pylint: disable=import-error from openedx.core.djangoapps.site_configuration import helpers +from .helpers import is_learner_records_enabled + API_VERSION = 'v2' @@ -97,7 +99,7 @@ class CredentialsApiConfig(ConfigurationModel): Publicly-accessible Records URL root. """ # Not every site wants the Learner Records feature, so we allow opting out. - if not helpers.get_value('ENABLE_LEARNER_RECORDS', True): + if not is_learner_records_enabled(): return None root = helpers.get_value('CREDENTIALS_PUBLIC_SERVICE_URL', settings.CREDENTIALS_PUBLIC_SERVICE_URL) return urljoin(root, '/records/') diff --git a/openedx/core/djangoapps/credentials/signals.py b/openedx/core/djangoapps/credentials/signals.py index 04f1b9705e..eb1dad9aaf 100644 --- a/openedx/core/djangoapps/credentials/signals.py +++ b/openedx/core/djangoapps/credentials/signals.py @@ -12,8 +12,8 @@ from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCer from lms.djangoapps.grades.api 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 .helpers import is_learner_records_enabled, is_learner_records_enabled_for_org from .tasks.v1.tasks import send_grade_to_credentials log = getLogger(__name__) @@ -74,7 +74,7 @@ def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, 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): + if not is_learner_records_enabled_for_org(course_run_key.org): if verbose: log.info( u"Skipping send grade: ENABLE_LEARNER_RECORDS False for org [{org}]".format( diff --git a/openedx/core/djangoapps/credentials/tests/test_signals.py b/openedx/core/djangoapps/credentials/tests/test_signals.py index de34014efe..314b7369f4 100644 --- a/openedx/core/djangoapps/credentials/tests/test_signals.py +++ b/openedx/core/djangoapps/credentials/tests/test_signals.py @@ -4,13 +4,14 @@ import ddt import mock from django.conf import settings -from django.test import TestCase +from django.test import TestCase, override_settings from opaque_keys.edx.keys import CourseKey from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory 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.helpers import is_learner_records_enabled 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 SiteConfigurationFactory, SiteFactory from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -124,6 +125,16 @@ class TestCredentialsSignalsSendGrade(TestCase): 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_records_disabled_globally( + self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials, + _mock_is_learner_issuance_enabled + ): + self.assertTrue(is_learner_records_enabled()) + with override_settings(FEATURES={"ENABLE_LEARNER_RECORDS": False}): + self.assertFalse(is_learner_records_enabled()) + 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') diff --git a/openedx/core/djangoapps/programs/signals.py b/openedx/core/djangoapps/programs/signals.py index 6a47ba00c1..60f25a7b54 100644 --- a/openedx/core/djangoapps/programs/signals.py +++ b/openedx/core/djangoapps/programs/signals.py @@ -6,13 +6,14 @@ This module contains signals / handlers related to programs. import logging from django.dispatch import receiver + +from openedx.core.djangoapps.credentials.helpers import is_learner_records_enabled_for_org from openedx.core.djangoapps.signals.signals import ( COURSE_CERT_AWARDED, COURSE_CERT_CHANGED, COURSE_CERT_DATE_CHANGE, COURSE_CERT_REVOKED ) -from openedx.core.djangoapps.site_configuration import helpers LOGGER = logging.getLogger(__name__) @@ -115,7 +116,7 @@ def handle_course_cert_changed(sender, user, course_key, mode, status, **kwargs) # 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): + if not is_learner_records_enabled_for_org(course_key.org): if verbose: LOGGER.info( u"Skipping send cert: ENABLE_LEARNER_RECORDS False for org [{org}]".format(