From 0137e38e970974fb6146ffa548f50db228fb9e55 Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Thu, 31 Aug 2017 14:03:54 -0400 Subject: [PATCH] EDUCATOR-1241 | make a separate file to make decisions about auto generated certs. --- cms/templates/settings.html | 4 +- lms/djangoapps/certificates/signals.py | 41 ++++-------- lms/djangoapps/courseware/date_summary.py | 9 ++- openedx/core/djangoapps/certificates/api.py | 39 +++++++++++ .../djangoapps/certificates/config/waffle.py | 1 + .../djangoapps/certificates/tests/test_api.py | 66 +++++++++++++++++++ 6 files changed, 123 insertions(+), 37 deletions(-) create mode 100644 openedx/core/djangoapps/certificates/api.py create mode 100644 openedx/core/djangoapps/certificates/tests/test_api.py diff --git a/cms/templates/settings.html b/cms/templates/settings.html index fdca1c4696..e98858e5ce 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -9,7 +9,7 @@ import urllib from django.utils.translation import ugettext as _ from contentstore import utils - from openedx.core.djangoapps.certificates.config import waffle + from openedx.core.djangoapps.certificates.api import can_show_certificate_available_date_field from openedx.core.djangolib.js_utils import ( dump_js_escaped_json, js_escaped_string ) @@ -214,7 +214,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}' - % if waffle.waffle().is_enabled(waffle.INSTRUCTOR_PACED_ONLY) and not context_course.self_paced: + % if can_show_certificate_available_date_field(context_course):
  1. diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index fd29abc4c4..7a3bd3c7ed 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -13,9 +13,13 @@ from certificates.models import ( GeneratedCertificate ) from certificates.tasks import generate_certificate -from courseware import courses 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.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 from student.models import CourseEnrollment @@ -25,21 +29,10 @@ 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 - switches = waffle.waffle() - # No flags enabled - if ( - not switches.is_enabled(waffle.SELF_PACED_ONLY) and - not switches.is_enabled(waffle.INSTRUCTOR_PACED_ONLY) - ): + course = CourseOverview.get_from_id(instance.course_id) + if not auto_certificate_generation_enabled_for_course(course): return - if courses.get_course_by_id(instance.course_id, depth=0).self_paced: - if not switches.is_enabled(waffle.SELF_PACED_ONLY): - return - else: - if not switches.is_enabled(waffle.INSTRUCTOR_PACED_ONLY): - return - fire_ungenerated_certificate_task(instance.user, instance.course_id) log.info(u'Certificate generation task initiated for {user} : {course} via whitelist'.format( user=instance.user.id, @@ -53,20 +46,10 @@ def _listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: dis Listen for a learner passing a course, send cert generation task, downstream signal from COURSE_GRADE_CHANGED """ - # No flags enabled - if ( - not waffle.waffle().is_enabled(waffle.SELF_PACED_ONLY) and - not waffle.waffle().is_enabled(waffle.INSTRUCTOR_PACED_ONLY) - ): + course = CourseOverview.get_from_id(course_id) + if not auto_certificate_generation_enabled_for_course(course): return - if courses.get_course_by_id(course_id, depth=0).self_paced: - if not waffle.waffle().is_enabled(waffle.SELF_PACED_ONLY): - return - else: - if not waffle.waffle().is_enabled(waffle.INSTRUCTOR_PACED_ONLY): - return - if fire_ungenerated_certificate_task(user, course_id): log.info(u'Certificate generation task initiated for {user} : {course} via passing grade'.format( user=user.id, @@ -80,11 +63,9 @@ def _listen_for_track_change(sender, user, **kwargs): # pylint: disable=unused- Catches a track change signal, determines user status, calls fire_ungenerated_certificate_task for passing grades """ - if ( - not waffle.waffle().is_enabled(waffle.SELF_PACED_ONLY) and - not waffle.waffle().is_enabled(waffle.INSTRUCTOR_PACED_ONLY) - ): + if not auto_certificate_generation_enabled(): return + user_enrollments = CourseEnrollment.enrollments_for_user(user=user) grade_factory = CourseGradeFactory() for enrollment in user_enrollments: diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index 8551053fc0..ba1e8479de 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -16,7 +16,7 @@ from pytz import timezone, utc from course_modes.models import CourseMode from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationDeadline -from openedx.core.djangoapps.certificates.config import waffle +from openedx.core.djangoapps.certificates.api import can_show_certificate_available_date_field from student.models import CourseEnrollment @@ -198,8 +198,8 @@ class CourseEndDate(DateSummary): class CertificateAvailableDate(DateSummary): """ - Displays the end date of the course. - """ + Displays the certificate available date of the course. + """ css_class = 'certificate-available-date' title = ugettext_lazy('Certificate Available') @@ -213,10 +213,9 @@ class CertificateAvailableDate(DateSummary): @property def is_enabled(self): return ( + can_show_certificate_available_date_field(self.course) and self.date is not None and datetime.datetime.now(utc) <= self.date and - not self.course.self_paced and - waffle.waffle().is_enabled(waffle.INSTRUCTOR_PACED_ONLY) and len(self.active_certificates) > 0 ) diff --git a/openedx/core/djangoapps/certificates/api.py b/openedx/core/djangoapps/certificates/api.py new file mode 100644 index 0000000000..b981046e54 --- /dev/null +++ b/openedx/core/djangoapps/certificates/api.py @@ -0,0 +1,39 @@ +""" +The public API for certificates. +""" + +from openedx.core.djangoapps.certificates.config import waffle + + +SWITCHES = waffle.waffle() + + +def auto_certificate_generation_enabled(): + return ( + SWITCHES.is_enabled(waffle.SELF_PACED_ONLY) or + SWITCHES.is_enabled(waffle.INSTRUCTOR_PACED_ONLY) + ) + + +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): + return not course.self_paced + return False + + +def can_show_certificate_available_date_field(course): + return _enabled_and_self_paced(course) diff --git a/openedx/core/djangoapps/certificates/config/waffle.py b/openedx/core/djangoapps/certificates/config/waffle.py index 52f69f04d5..eb4f4fb6ab 100644 --- a/openedx/core/djangoapps/certificates/config/waffle.py +++ b/openedx/core/djangoapps/certificates/config/waffle.py @@ -8,6 +8,7 @@ from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace 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' diff --git a/openedx/core/djangoapps/certificates/tests/test_api.py b/openedx/core/djangoapps/certificates/tests/test_api.py new file mode 100644 index 0000000000..75f23eb832 --- /dev/null +++ b/openedx/core/djangoapps/certificates/tests/test_api.py @@ -0,0 +1,66 @@ +from contextlib import contextmanager +import itertools +from unittest import TestCase + +import ddt +import waffle + +from openedx.core.djangoapps.certificates import api +from openedx.core.djangoapps.certificates.config import waffle as certs_waffle +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory + + +@contextmanager +def configure_waffle_namespace(self_paced_enabled, instructor_paced_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): + yield + + +@ddt.ddt +class FeatureEnabledTestCase(TestCase): + def setUp(self): + super(FeatureEnabledTestCase, self).setUp() + self.course = CourseOverviewFactory.create() + + def tearDown(self): + 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( + (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 + ) + @ddt.unpack + def test_can_show_certificate_available_date_field( + 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.can_show_certificate_available_date_field(self.course))