From 6a0f57a266945436f48cc9c29a0826d2d82d73c3 Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Wed, 2 Jun 2021 15:14:52 -0400 Subject: [PATCH] refactor: update references to deprecated whitelist model [MICROBA-1012] - update python files to reference the allowlist instead of whitelist As part of the Certificates v2 work, and in an effort to make the codebase more inclusive, we are moving away from using the term "whitelist" in favor of "allowlist". This PR is part of our renaming efforts across the Certificates Django app, and other Django apps that make use of Certificates functionality. --- lms/djangoapps/certificates/models.py | 4 +- lms/djangoapps/certificates/queue.py | 14 +++--- lms/djangoapps/certificates/signals.py | 8 ++-- .../tests/test_generation_handler.py | 6 +-- .../certificates/tests/test_models.py | 4 +- .../certificates/tests/test_queue.py | 4 +- lms/djangoapps/certificates/tests/tests.py | 46 +++++++++---------- .../instructor/tests/test_certificates.py | 4 +- lms/djangoapps/instructor/views/api.py | 8 ++-- lms/djangoapps/instructor_task/api.py | 4 +- .../instructor_task/tasks_helper/certs.py | 4 +- .../tests/test_tasks_helper.py | 6 +-- 12 files changed, 56 insertions(+), 56 deletions(-) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 645cd00179..6272815104 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -748,7 +748,7 @@ def certificate_status(generated_certificate): return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor, 'uuid': None} -def certificate_info_for_user(user, course_id, grade, user_is_whitelisted, user_certificate): +def certificate_info_for_user(user, course_id, grade, user_is_allowlisted, user_certificate): """ Returns the certificate info for a user for grade report. """ @@ -763,7 +763,7 @@ def certificate_info_for_user(user, course_id, grade, user_is_whitelisted, user_ mode_is_verified = enrollment_mode in CourseMode.VERIFIED_MODES user_is_verified = grade is not None and mode_is_verified - eligible_for_certificate = 'Y' if (user_is_whitelisted or user_is_verified or certificate_generated) \ + eligible_for_certificate = 'Y' if (user_is_allowlisted or user_is_verified or certificate_generated) \ else 'N' if certificate_generated and can_have_certificate: diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index de1aa3cc3c..395c9b90c6 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -103,7 +103,7 @@ class XQueueCertInterface: settings.XQUEUE_INTERFACE['django_auth'], requests_auth, ) - self.whitelist = CertificateWhitelist.objects.all() + self.allowlist = CertificateWhitelist.objects.all() self.use_https = True def regen_cert(self, student, course_id, forced_grade=None, template_file=None, generate_pdf=True): @@ -194,7 +194,7 @@ class XQueueCertInterface: Certificate must be in the 'unavailable', 'error', 'deleted' or 'generating' state. - If a student has a passing grade or is in the whitelist + If a student has a passing grade or is in the allowlist table for the course a request will be made for a new cert. If a student does not have a passing grade the status @@ -259,7 +259,7 @@ class XQueueCertInterface: self.request.user = student self.request.session = {} - is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists() + is_allowlisted = self.allowlist.filter(user=student, course_id=course_id, whitelist=True).exists() course_grade = CourseGradeFactory().read(student, course_key=course_id) enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id) mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES @@ -267,7 +267,7 @@ class XQueueCertInterface: cert_mode = enrollment_mode is_eligible_for_certificate = modes_api.is_eligible_for_certificate(enrollment_mode, cert_status) - if is_whitelisted and not is_eligible_for_certificate: + if is_allowlisted and not is_eligible_for_certificate: # check if audit certificates are enabled for audit mode is_eligible_for_certificate = enrollment_mode != CourseMode.AUDIT or \ not settings.FEATURES['DISABLE_AUDIT_CERTIFICATES'] @@ -337,10 +337,10 @@ class XQueueCertInterface: str(exc) ) - # Check if the student is whitelisted - if is_whitelisted: + # Check if the student is on the allowlist + if is_allowlisted: LOGGER.info( - "Student %s is whitelisted in '%s'", + "Student %s is on the certificate allowlist in '%s'", student.id, str(course_id) ) diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index d0237eb67f..f0530a92a7 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -52,10 +52,10 @@ def _update_cert_settings_on_pacing_change(sender, updated_course_overview, **kw )) -@receiver(post_save, sender=CertificateWhitelist, dispatch_uid="append_certificate_whitelist") -def _listen_for_certificate_whitelist_append(sender, instance, **kwargs): # pylint: disable=unused-argument +@receiver(post_save, sender=CertificateWhitelist, dispatch_uid="append_certificate_allowlist") +def _listen_for_certificate_allowlist_append(sender, instance, **kwargs): # pylint: disable=unused-argument """ - Listen for a user being added to or modified on the whitelist (allowlist) + Listen for a user being added to or modified on the allowlist """ if not auto_certificate_generation_enabled(): return @@ -66,7 +66,7 @@ def _listen_for_certificate_whitelist_append(sender, instance, **kwargs): # pyl return generate_allowlist_certificate_task(instance.user, instance.course_id) if _fire_ungenerated_certificate_task(instance.user, instance.course_id): - log.info('Certificate generation task initiated for {user} : {course} via whitelist'.format( + log.info('Certificate generation task initiated for {user} : {course} via allowlist'.format( user=instance.user.id, course=instance.course_id )) diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index cb73efe86b..2e5c436864 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -62,7 +62,7 @@ class AllowlistTests(ModuleStoreTestCase): mode="verified", ) - # Whitelist user + # Add user to the allowlist CertificateAllowlistFactory.create(course_id=self.course_run_key, user=self.user) def test_is_on_allowlist(self): @@ -186,9 +186,9 @@ class AllowlistTests(ModuleStoreTestCase): assert not _can_generate_allowlist_certificate(u, key) assert _set_allowlist_cert_status(u, key) is None - def test_can_generate_not_whitelisted(self): + def test_can_generate_not_allowlisted(self): """ - Test handling when user is not whitelisted + Test handling when user is not on the certificate allowlist. """ u = UserFactory() cr = CourseFactory() diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py index 6c90f93927..8b346d2de1 100644 --- a/lms/djangoapps/certificates/tests/test_models.py +++ b/lms/djangoapps/certificates/tests/test_models.py @@ -240,8 +240,8 @@ class TestCertificateGenerationHistory(TestCase): Test the CertificateGenerationHistory model's methods """ @ddt.data( - ({"student_set": "whitelisted_not_generated"}, "For exceptions", True), - ({"student_set": "whitelisted_not_generated"}, "For exceptions", False), + ({"student_set": "allowlisted_not_generated"}, "For exceptions", True), + ({"student_set": "allowlisted_not_generated"}, "For exceptions", False), # check "students" key for backwards compatibility ({"students": [1, 2, 3]}, "For exceptions", True), ({"students": [1, 2, 3]}, "For exceptions", False), diff --git a/lms/djangoapps/certificates/tests/test_queue.py b/lms/djangoapps/certificates/tests/test_queue.py index c5c282bef0..6522bffa6d 100644 --- a/lms/djangoapps/certificates/tests/test_queue.py +++ b/lms/djangoapps/certificates/tests/test_queue.py @@ -112,7 +112,7 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): @ddt.data((True, CertificateStatuses.audit_passing), (False, CertificateStatuses.generating)) @ddt.unpack @override_settings(AUDIT_CERT_CUTOFF_DATE=datetime.now(pytz.UTC) - timedelta(days=1)) - def test_ineligible_cert_whitelisted(self, disable_audit_cert, status): + def test_ineligible_cert_allowlisted(self, disable_audit_cert, status): """ Test that audit mode students receive a certificate if DISABLE_AUDIT_CERTIFICATES feature is set to false @@ -124,7 +124,7 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): is_active=True, mode='audit' ) - # Whitelist student + # Add student to the allowlist CertificateAllowlistFactory(course_id=self.course.id, user=self.user_2) features = settings.FEATURES diff --git a/lms/djangoapps/certificates/tests/tests.py b/lms/djangoapps/certificates/tests/tests.py index afb2df0605..4a343b69a5 100644 --- a/lms/djangoapps/certificates/tests/tests.py +++ b/lms/djangoapps/certificates/tests/tests.py @@ -58,13 +58,13 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin): @unpack @data( - {'whitelisted': False, 'grade': None, 'output': ['N', 'N', 'N/A']}, - {'whitelisted': True, 'grade': None, 'output': ['Y', 'N', 'N/A']}, - {'whitelisted': False, 'grade': 0.9, 'output': ['N', 'N', 'N/A']}, - {'whitelisted': True, 'grade': 0.8, 'output': ['Y', 'N', 'N/A']}, - {'whitelisted': None, 'grade': 0.8, 'output': ['N', 'N', 'N/A']} + {'allowlisted': False, 'grade': None, 'output': ['N', 'N', 'N/A']}, + {'allowlisted': True, 'grade': None, 'output': ['Y', 'N', 'N/A']}, + {'allowlisted': False, 'grade': 0.9, 'output': ['N', 'N', 'N/A']}, + {'allowlisted': True, 'grade': 0.8, 'output': ['Y', 'N', 'N/A']}, + {'allowlisted': None, 'grade': 0.8, 'output': ['N', 'N', 'N/A']} ) - def test_certificate_info_for_user(self, whitelisted, grade, output): + def test_certificate_info_for_user(self, allowlisted, grade, output): """ Verify that certificate_info_for_user works. """ @@ -73,28 +73,28 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin): # for instructor paced course certificate_info = certificate_info_for_user( student, self.instructor_paced_course.id, grade, - whitelisted, user_certificate=None + allowlisted, user_certificate=None ) assert certificate_info == output # for self paced course certificate_info = certificate_info_for_user( student, self.self_paced_course.id, grade, - whitelisted, user_certificate=None + allowlisted, user_certificate=None ) assert certificate_info == output @unpack @data( - {'whitelisted': False, 'grade': None, 'output': ['Y', 'Y', 'honor']}, - {'whitelisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']}, - {'whitelisted': False, 'grade': 0.9, 'output': ['Y', 'Y', 'honor']}, - {'whitelisted': True, 'grade': 0.8, 'output': ['Y', 'Y', 'honor']}, - {'whitelisted': None, 'grade': 0.8, 'output': ['Y', 'Y', 'honor']}, - {'whitelisted': None, 'grade': None, 'output': ['Y', 'Y', 'honor']}, - {'whitelisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']} + {'allowlisted': False, 'grade': None, 'output': ['Y', 'Y', 'honor']}, + {'allowlisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']}, + {'allowlisted': False, 'grade': 0.9, 'output': ['Y', 'Y', 'honor']}, + {'allowlisted': True, 'grade': 0.8, 'output': ['Y', 'Y', 'honor']}, + {'allowlisted': None, 'grade': 0.8, 'output': ['Y', 'Y', 'honor']}, + {'allowlisted': None, 'grade': None, 'output': ['Y', 'Y', 'honor']}, + {'allowlisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']} ) - def test_certificate_info_for_user_when_grade_changes(self, whitelisted, grade, output): + def test_certificate_info_for_user_when_grade_changes(self, allowlisted, grade, output): """ Verify that certificate_info_for_user works as expect in scenario when grading of problems changes after certificates already generated. In such scenario `Certificate delivered` should not depend @@ -120,24 +120,24 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin): # for instructor paced course certificate_info = certificate_info_for_user( student, self.instructor_paced_course.id, grade, - whitelisted, certificate1 + allowlisted, certificate1 ) assert certificate_info == output # for self paced course certificate_info = certificate_info_for_user( student, self.self_paced_course.id, grade, - whitelisted, certificate2 + allowlisted, certificate2 ) assert certificate_info == output @unpack @data( - {'whitelisted': False, 'grade': 0.8, 'mode': 'audit', 'output': ['N', 'N', 'N/A']}, - {'whitelisted': True, 'grade': 0.8, 'mode': 'audit', 'output': ['Y', 'N', 'N/A']}, - {'whitelisted': False, 'grade': 0.8, 'mode': 'verified', 'output': ['Y', 'N', 'N/A']} + {'allowlisted': False, 'grade': 0.8, 'mode': 'audit', 'output': ['N', 'N', 'N/A']}, + {'allowlisted': True, 'grade': 0.8, 'mode': 'audit', 'output': ['Y', 'N', 'N/A']}, + {'allowlisted': False, 'grade': 0.8, 'mode': 'verified', 'output': ['Y', 'N', 'N/A']} ) - def test_certificate_info_for_user_with_course_modes(self, whitelisted, grade, mode, output): + def test_certificate_info_for_user_with_course_modes(self, allowlisted, grade, mode, output): """ Verify that certificate_info_for_user works with course modes. """ @@ -146,7 +146,7 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin): _ = CourseEnrollment.enroll(user, self.instructor_paced_course.id, mode) certificate_info = certificate_info_for_user( user, self.instructor_paced_course.id, grade, - whitelisted, user_certificate=None + allowlisted, user_certificate=None ) assert certificate_info == output diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index 752c13f53b..15ccfb6110 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -831,7 +831,7 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): # Assert Request is successful assert res_json['success'] # Assert Message - assert res_json['message'] == 'Certificate generation started for white listed students.' + assert res_json['message'] == 'Certificate generation started for students on the allowlist.' def test_generate_certificate_exceptions_allowlist_not_generated(self): """ @@ -856,7 +856,7 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): # Assert Request is successful assert res_json['success'] # Assert Message - assert res_json['message'] == 'Certificate generation started for white listed students.' + assert res_json['message'] == 'Certificate generation started for students on the allowlist.' def test_generate_certificate_exceptions_generate_for_incorrect_value(self): """ diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 0cbfa85be7..ba7f6df94d 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3123,10 +3123,10 @@ def generate_certificate_exceptions(request, course_id, generate_for=None): if generate_for == 'all': # Generate Certificates for all allowlisted students - students = 'all_whitelisted' + students = 'all_allowlisted' elif generate_for == 'new': - students = 'whitelisted_not_generated' + students = 'allowlisted_not_generated' else: # Invalid data, generate_for must be present for all certificate exceptions @@ -3141,7 +3141,7 @@ def generate_certificate_exceptions(request, course_id, generate_for=None): task_api.generate_certificates_for_students(request, course_key, student_set=students) response_payload = { 'success': True, - 'message': _('Certificate generation started for white listed students.'), + 'message': _('Certificate generation started for students on the allowlist.'), } return JsonResponse(response_payload) @@ -3273,7 +3273,7 @@ def certificate_invalidation_view(request, course_id): try: if certs_api.is_on_allowlist(student, course_key): log.warning(f"Invalidating certificate for student {student.id} in course {course_key} failed. " - "Student is currently on the allow list.") + "Student is currently on the allowlist.") raise ValueError( _("The student {student} appears on the Certificate Exception list in course {course}. Please " "remove them from the Certificate Exception list before attempting to invalidate their " diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 718b202997..ae57a9396c 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -485,8 +485,8 @@ def generate_certificates_for_students(request, course_key, student_set=None, sp course_key : Course Key student_set : Semantic for student collection for certificate generation. Options are: - 'all_whitelisted': All Whitelisted students. - 'whitelisted_not_generated': Whitelisted students which does not got certificates yet. + 'all_allowlisted': All students on the certificate allowlist. + 'allowlisted_not_generated': Students on certificate allowlist who do not have certificates yet. 'specific_student': Single student for certificate generation. specific_student_id : Student ID when student_set is 'specific_student' diff --git a/lms/djangoapps/instructor_task/tasks_helper/certs.py b/lms/djangoapps/instructor_task/tasks_helper/certs.py index 1bd686e67c..bd54bbc24c 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/certs.py +++ b/lms/djangoapps/instructor_task/tasks_helper/certs.py @@ -39,11 +39,11 @@ def generate_students_certificates( students_to_generate_certs_for = CourseEnrollment.objects.users_enrolled_in(course_id) student_set = task_input.get('student_set') - if student_set == 'all_whitelisted': + if student_set == 'all_allowlisted': # Generate Certificates for all allowlisted students. students_to_generate_certs_for = get_enrolled_allowlisted_users(course_id) - elif student_set == 'whitelisted_not_generated': + elif student_set == 'allowlisted_not_generated': # Allowlisted students who did not yet receive certificates students_to_generate_certs_for = get_enrolled_allowlisted_not_passing_users(course_id) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index d6dc29f21f..d506e9d1a1 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -2064,7 +2064,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): status=status, ) - task_input = {'student_set': 'all_whitelisted'} + task_input = {'student_set': 'all_allowlisted'} # Only certificates for the 3 allowlisted students should have been run expected_results = { @@ -2106,7 +2106,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): user=student, course_id=self.course.id ) - task_input = {'student_set': 'whitelisted_not_generated'} + task_input = {'student_set': 'allowlisted_not_generated'} # Certificates should only be generated for the allowlisted students # who do not yet have passing certificates. @@ -2419,7 +2419,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Certificates should be regenerated for students having generated certificates with status # 'downloadable' or 'error' which are total of 5 students in this test case - task_input = {'student_set': "all_whitelisted"} + task_input = {'student_set': "all_allowlisted"} expected_results = { 'action_name': 'certificates generated',