From e4735a6abab8704cbd1e1ef614ac71516c79ccc3 Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Wed, 13 Sep 2017 12:28:04 -0400 Subject: [PATCH] switch over to unified waffle switch for EDUCATOR-1313 --- lms/djangoapps/certificates/signals.py | 9 ++-- .../certificates/tests/test_signals.py | 20 ++++----- .../certificates/tests/test_webview_views.py | 5 +-- .../courseware/tests/test_date_summary.py | 7 ++-- lms/djangoapps/courseware/views/views.py | 5 +-- openedx/core/djangoapps/certificates/api.py | 27 +++--------- .../djangoapps/certificates/config/waffle.py | 2 - .../djangoapps/certificates/tests/test_api.py | 42 ++++++------------- 8 files changed, 37 insertions(+), 80 deletions(-) diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 7a3bd3c7ed..08971de42f 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -14,10 +14,7 @@ from certificates.models import ( ) from certificates.tasks import generate_certificate from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory -from openedx.core.djangoapps.certificates.api import ( - auto_certificate_generation_enabled, - auto_certificate_generation_enabled_for_course, -) +from openedx.core.djangoapps.certificates.api import auto_certificate_generation_enabled from openedx.core.djangoapps.certificates.config import waffle from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED, LEARNER_NOW_VERIFIED @@ -30,7 +27,7 @@ log = logging.getLogger(__name__) @receiver(post_save, sender=CertificateWhitelist, dispatch_uid="append_certificate_whitelist") def _listen_for_certificate_whitelist_append(sender, instance, **kwargs): # pylint: disable=unused-argument course = CourseOverview.get_from_id(instance.course_id) - if not auto_certificate_generation_enabled_for_course(course): + if not auto_certificate_generation_enabled(): return fire_ungenerated_certificate_task(instance.user, instance.course_id) @@ -47,7 +44,7 @@ def _listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: dis downstream signal from COURSE_GRADE_CHANGED """ course = CourseOverview.get_from_id(course_id) - if not auto_certificate_generation_enabled_for_course(course): + if not auto_certificate_generation_enabled(): return if fire_ungenerated_certificate_task(user, course_id): diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 7712cf1922..77fea0e382 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -74,19 +74,19 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase): def test_cert_generation_on_whitelist_append_self_paced(self): """ Verify that signal is sent, received, and fires task - based on 'SELF_PACED_ONLY' flag + based on 'AUTO_CERTIFICATE_GENERATION' flag """ with mock.patch( 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', return_value=None ) as mock_generate_certificate_apply_async: - with waffle.waffle().override(waffle.SELF_PACED_ONLY, active=False): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=False): CertificateWhitelist.objects.create( user=self.user, course_id=self.course.id ) mock_generate_certificate_apply_async.assert_not_called() - with waffle.waffle().override(waffle.SELF_PACED_ONLY, active=True): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=True): CertificateWhitelist.objects.create( user=self.user, course_id=self.course.id @@ -99,19 +99,19 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase): def test_cert_generation_on_whitelist_append_instructor_paced(self): """ Verify that signal is sent, received, and fires task - based on 'INSTRUCTOR_PACED_ONLY' flag + based on 'AUTO_CERTIFICATE_GENERATION' flag """ with mock.patch( 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', return_value=None ) as mock_generate_certificate_apply_async: - with waffle.waffle().override(waffle.INSTRUCTOR_PACED_ONLY, active=False): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=False): CertificateWhitelist.objects.create( user=self.user, course_id=self.ip_course.id ) mock_generate_certificate_apply_async.assert_not_called() - with waffle.waffle().override(waffle.INSTRUCTOR_PACED_ONLY, active=True): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=True): CertificateWhitelist.objects.create( user=self.user, course_id=self.ip_course.id @@ -156,7 +156,7 @@ class PassingGradeCertsTest(ModuleStoreTestCase): 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', return_value=None ) as mock_generate_certificate_apply_async: - with waffle.waffle().override(waffle.SELF_PACED_ONLY, active=True): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=True): grade_factory = CourseGradeFactory() # Not passing grade_factory.update(self.user, self.course) @@ -174,7 +174,7 @@ class PassingGradeCertsTest(ModuleStoreTestCase): 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', return_value=None ) as mock_generate_certificate_apply_async: - with waffle.waffle().override(waffle.INSTRUCTOR_PACED_ONLY, active=True): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=True): grade_factory = CourseGradeFactory() # Not passing grade_factory.update(self.user, self.ip_course) @@ -237,7 +237,7 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase): 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', return_value=None ) as mock_generate_certificate_apply_async: - with waffle.waffle().override(waffle.SELF_PACED_ONLY, active=True): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=True): mock_generate_certificate_apply_async.assert_not_called() attempt = SoftwareSecurePhotoVerification.objects.create( user=self.user_one, @@ -254,7 +254,7 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase): 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', return_value=None ) as mock_generate_certificate_apply_async: - with waffle.waffle().override(waffle.INSTRUCTOR_PACED_ONLY, active=True): + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=True): mock_generate_certificate_apply_async.assert_not_called() attempt = SoftwareSecurePhotoVerification.objects.create( user=self.user_two, diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 179c597926..e28529a83b 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -820,9 +820,8 @@ class CertificatesViewsTests(CommonCertificatesTestCase): expected_date = today else: expected_date = self.course.certificate_available_date - with waffle.waffle().override(waffle.SELF_PACED_ONLY, active=True): - with waffle.waffle().override(waffle.INSTRUCTOR_PACED_ONLY, active=True): - response = self.client.get(test_url) + with waffle.waffle().override(waffle.AUTO_CERTIFICATE_GENERATION, active=True): + response = self.client.get(test_url) date = '{month} {day}, {year}'.format( month=strftime_localized(expected_date, "%B"), day=expected_date.day, diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index bf8cb6595c..b9ccee892d 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -329,7 +329,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): self.assertEqual(block.link, '{}?sku={}'.format(configuration.MULTIPLE_ITEMS_BASKET_PAGE_URL, sku)) ## CertificateAvailableDate - @waffle.testutils.override_switch('certificates.instructor_paced_only', True) + @waffle.testutils.override_switch('certificates.auto_certificate_generation', True) def test_no_certificate_available_date(self): course = create_course_run(days_till_start=-1) user = self.create_user() @@ -339,7 +339,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): self.assertFalse(block.is_enabled) ## CertificateAvailableDate - @waffle.testutils.override_switch('certificates.instructor_paced_only', True) + @waffle.testutils.override_switch('certificates.auto_certificate_generation', True) def test_no_certificate_available_date_for_self_paced(self): course = create_self_paced_course_run() verified_user = self.create_user() @@ -350,7 +350,6 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): self.assertNotEqual(block.date, None) self.assertFalse(block.is_enabled) - # @waffle.testutils.override_switch('certificates.instructor_paced_only', True) def test_no_certificate_available_date_for_audit_course(self): """ Tests that Certificate Available Date is not visible in the course "Important Course Dates" section @@ -374,7 +373,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): self.assertFalse(block.is_enabled) self.assertNotEqual(block.date, None) - @waffle.testutils.override_switch('certificates.instructor_paced_only', True) + @waffle.testutils.override_switch('certificates.auto_certificate_generation', True) def test_certificate_available_date_defined(self): course = create_course_run() audit_user = self.create_user() diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 78f96c153d..47faebccd4 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -933,9 +933,8 @@ def _get_cert_data(student, course, course_key, is_active, enrollment_mode, grad may_view_certificate = get_course_by_id(course_key).may_certify() switches = certificates_waffle.waffle() - switches_enabled = (switches.is_enabled(certificates_waffle.SELF_PACED_ONLY) and - switches.is_enabled(certificates_waffle.INSTRUCTOR_PACED_ONLY)) - student_cert_generation_enabled = certs_api.cert_generation_enabled(course_key) if not switches_enabled else True + switch_enabled = switches.is_enabled(certificates_waffle.AUTO_CERTIFICATE_GENERATION) + student_cert_generation_enabled = switch_enabled or certs_api.cert_generation_enabled(course_key) # Don't show certificate information if: # 1) the learner has not passed the course diff --git a/openedx/core/djangoapps/certificates/api.py b/openedx/core/djangoapps/certificates/api.py index 21b015c03d..2666d98e9e 100644 --- a/openedx/core/djangoapps/certificates/api.py +++ b/openedx/core/djangoapps/certificates/api.py @@ -10,39 +10,22 @@ SWITCHES = waffle.waffle() def auto_certificate_generation_enabled(): - return ( - SWITCHES.is_enabled(waffle.SELF_PACED_ONLY) or - SWITCHES.is_enabled(waffle.INSTRUCTOR_PACED_ONLY) - ) + return SWITCHES.is_enabled(waffle.AUTO_CERTIFICATE_GENERATION) -def auto_certificate_generation_enabled_for_course(course): - if not auto_certificate_generation_enabled(): - return False - - if course.self_paced: - if not SWITCHES.is_enabled(waffle.SELF_PACED_ONLY): - return False - else: - if not SWITCHES.is_enabled(waffle.INSTRUCTOR_PACED_ONLY): - return False - - return True - - -def _enabled_and_self_paced(course): - if auto_certificate_generation_enabled_for_course(course): +def _enabled_and_instructor_paced(course): + if auto_certificate_generation_enabled(): return not course.self_paced return False def can_show_certificate_available_date_field(course): - return _enabled_and_self_paced(course) + return _enabled_and_instructor_paced(course) def display_date_for_certificate(course, certificate): if ( - auto_certificate_generation_enabled_for_course(course) and + auto_certificate_generation_enabled() and not course.self_paced and course.certificate_available_date and course.certificate_available_date < datetime.now(UTC) diff --git a/openedx/core/djangoapps/certificates/config/waffle.py b/openedx/core/djangoapps/certificates/config/waffle.py index eb4f4fb6ab..ffcf09a78d 100644 --- a/openedx/core/djangoapps/certificates/config/waffle.py +++ b/openedx/core/djangoapps/certificates/config/waffle.py @@ -9,8 +9,6 @@ WAFFLE_NAMESPACE = u'certificates' # Switches AUTO_CERTIFICATE_GENERATION = u'auto_certificate_generation' -SELF_PACED_ONLY = u'self_paced_only' -INSTRUCTOR_PACED_ONLY = u'instructor_paced_only' def waffle(): diff --git a/openedx/core/djangoapps/certificates/tests/test_api.py b/openedx/core/djangoapps/certificates/tests/test_api.py index 75f23eb832..642be50a3a 100644 --- a/openedx/core/djangoapps/certificates/tests/test_api.py +++ b/openedx/core/djangoapps/certificates/tests/test_api.py @@ -11,11 +11,10 @@ from openedx.core.djangoapps.content.course_overviews.tests.factories import Cou @contextmanager -def configure_waffle_namespace(self_paced_enabled, instructor_paced_enabled): +def configure_waffle_namespace(feature_enabled): namespace = certs_waffle.waffle() - with namespace.override(certs_waffle.SELF_PACED_ONLY, active=self_paced_enabled): - with namespace.override(certs_waffle.INSTRUCTOR_PACED_ONLY, active=instructor_paced_enabled): + with namespace.override(certs_waffle.AUTO_CERTIFICATE_GENERATION, active=feature_enabled): yield @@ -29,38 +28,21 @@ class FeatureEnabledTestCase(TestCase): super(FeatureEnabledTestCase, self).tearDown() self.course.self_paced = False - @ddt.data(*itertools.product((True, False), (True, False))) - @ddt.unpack - def test_auto_certificate_generation_enabled(self, self_paced_enabled, instructor_paced_enabled): - expected_value = self_paced_enabled or instructor_paced_enabled - with configure_waffle_namespace(self_paced_enabled, instructor_paced_enabled): - self.assertEqual(expected_value, api.auto_certificate_generation_enabled()) + @ddt.data(True, False) + def test_auto_certificate_generation_enabled(self, feature_enabled): + with configure_waffle_namespace(feature_enabled): + self.assertEqual(feature_enabled, api.auto_certificate_generation_enabled()) @ddt.data( - (False, False, True, False), # feature not enabled should return False - (False, True, True, False), # self-paced feature enabled and self-paced course should return False - (True, False, True, True), # self-paced feature enabled and self-paced course should return True - (True, False, False, False), # instructor-paced feature enabled and self-paced course should return False - (False, True, False, True) # instructor-paced feature enabled and instructor-paced course should return True - ) - @ddt.unpack - def test_auto_certificate_generation_enabled_for_course( - self, self_paced_enabled, instructor_paced_enabled, is_self_paced, expected_value - ): - self.course.self_paced = is_self_paced - with configure_waffle_namespace(self_paced_enabled, instructor_paced_enabled): - self.assertEqual(expected_value, api.auto_certificate_generation_enabled_for_course(self.course)) - - @ddt.data( - (True, False, True, False), # feature enabled and self-paced should return False - (False, True, False, True), # feature enabled and instructor-paced should return True - (False, False, True, False), # feature not enabled and self-paced should return False - (False, False, False, False), # feature not enabled and instructor-paced should return False + (True, True, False), # feature enabled and self-paced should return False + (True, False, True), # feature enabled and instructor-paced should return True + (False, True, False), # feature not enabled and self-paced should return False + (False, False, False), # feature not enabled and instructor-paced should return False ) @ddt.unpack def test_can_show_certificate_available_date_field( - self, self_paced_enabled, instructor_paced_enabled, is_self_paced, expected_value + self, feature_enabled, is_self_paced, expected_value ): self.course.self_paced = is_self_paced - with configure_waffle_namespace(self_paced_enabled, instructor_paced_enabled): + with configure_waffle_namespace(feature_enabled): self.assertEqual(expected_value, api.can_show_certificate_available_date_field(self.course))