From 1473973b2b0df9c75a70d24294f155eae8ff331f Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Wed, 12 May 2021 09:51:26 -0400 Subject: [PATCH] refactor: Rename whitelist to allowlist (#27533) MICROBA-1021 --- lms/djangoapps/certificates/api.py | 28 +++++++++++++++++ .../certificates/tests/test_signals.py | 6 ++-- lms/djangoapps/courseware/tests/test_views.py | 19 ++++++------ .../instructor_task/tasks_helper/certs.py | 19 ++++-------- .../instructor_task/tasks_helper/grades.py | 11 ++++--- .../tests/test_tasks_helper.py | 31 +++++++++---------- 6 files changed, 66 insertions(+), 48 deletions(-) diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 1e4ed3979d..b86a05d8da 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -20,6 +20,7 @@ from opaque_keys.edx.django.models import CourseKeyField from organizations.api import get_course_organization_id from common.djangoapps.student.api import is_user_enrolled_in_course +from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.branding import api as branding_api from lms.djangoapps.certificates.generation_handler import ( can_generate_certificate_task as _can_generate_certificate_task, @@ -741,3 +742,30 @@ def get_allowlist(course_key): Return the certificate allowlist for the given course run """ return CertificateWhitelist.get_certificate_white_list(course_key) + + +def get_enrolled_allowlisted_users(course_key): + """ + Get all users who: + - are enrolled in this course run + - are allowlisted in this course run + """ + users = CourseEnrollment.objects.users_enrolled_in(course_key) + return users.filter( + certificatewhitelist__course_id=course_key, + certificatewhitelist__whitelist=True + ) + + +def get_enrolled_allowlisted_not_passing_users(course_key): + """ + Get all users who: + - are enrolled in this course run + - are allowlisted in this course run + - do not have a course certificate with a passing status + """ + users = get_enrolled_allowlisted_users(course_key) + return users.exclude( + generatedcertificate__course_id=course_key, + generatedcertificate__status__in=CertificateStatuses.PASSED_STATUSES + ) diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 14e98e40b3..b647189a97 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -90,8 +90,7 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): CertificateAllowlistFactory( user=self.user, - course_id=self.ip_course.id, - whitelist=True + course_id=self.ip_course.id ) mock_generate_certificate_apply_async.assert_not_called() mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id) @@ -111,8 +110,7 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False): CertificateAllowlistFactory( user=self.user, - course_id=self.ip_course.id, - whitelist=True + course_id=self.ip_course.id ) mock_generate_certificate_apply_async.assert_not_called() mock_generate_allowlist_task.assert_not_called() diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 3a9965fbbd..cec4161402 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -40,10 +40,13 @@ from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, - CertificateStatuses, - CertificateWhitelist + CertificateStatuses +) +from lms.djangoapps.certificates.tests.factories import ( + CertificateAllowlistFactory, + CertificateInvalidationFactory, + GeneratedCertificateFactory ) -from lms.djangoapps.certificates.tests.factories import CertificateInvalidationFactory, GeneratedCertificateFactory from lms.djangoapps.commerce.models import CommerceConfiguration from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.courseware.access_utils import check_course_open_for_learner @@ -1622,10 +1625,9 @@ class ProgressPageTests(ProgressPageBaseTests): self.assert_invalidate_certificate(generated_certificate) @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) - def test_page_with_whitelisted_certificate_with_html_view(self): + def test_page_with_allowlisted_certificate_with_html_view(self): """ - Verify that for white listed user the view certificate is - appearing on dashboard + Verify that view certificate appears for an allowlisted user """ generated_certificate = self.generate_certificate( "http://www.example.com/certificate.pdf", "honor" @@ -1647,10 +1649,9 @@ class ProgressPageTests(ProgressPageBaseTests): self.course.cert_html_view_enabled = True self.course.save() self.store.update_item(self.course, self.user.id) - CertificateWhitelist.objects.create( + CertificateAllowlistFactory.create( user=self.user, - course_id=self.course.id, - whitelist=True + course_id=self.course.id ) with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: diff --git a/lms/djangoapps/instructor_task/tasks_helper/certs.py b/lms/djangoapps/instructor_task/tasks_helper/certs.py index d9299b9c4c..b41bc63cc1 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/certs.py +++ b/lms/djangoapps/instructor_task/tasks_helper/certs.py @@ -16,6 +16,8 @@ from lms.djangoapps.certificates.api import ( generate_certificate_task, generate_user_certificates, get_allowlisted_users, + get_enrolled_allowlisted_users, + get_enrolled_allowlisted_not_passing_users, is_using_v2_course_certificates, ) from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate @@ -39,21 +41,12 @@ def generate_students_certificates( student_set = task_input.get('student_set') if student_set == 'all_whitelisted': - # Generate Certificates for all white listed students. - students_to_generate_certs_for = students_to_generate_certs_for.filter( - certificatewhitelist__course_id=course_id, - certificatewhitelist__whitelist=True - ) + # Generate Certificates for all allowlisted students. + students_to_generate_certs_for = get_enrolled_allowlisted_users(course_id) elif student_set == 'whitelisted_not_generated': - # Whitelist students who did not get certificates already. - students_to_generate_certs_for = students_to_generate_certs_for.filter( - certificatewhitelist__course_id=course_id, - certificatewhitelist__whitelist=True - ).exclude( - generatedcertificate__course_id=course_id, - generatedcertificate__status__in=CertificateStatuses.PASSED_STATUSES - ) + # Allowlisted students who did not yet receive certificates + students_to_generate_certs_for = get_enrolled_allowlisted_not_passing_users(course_id) elif student_set == "specific_student": specific_student_id = task_input.get('specific_student_id') diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index 59e7e2972c..a25b1c1cb5 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -19,7 +19,8 @@ from six.moves import zip_longest from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import BulkRoleCache -from lms.djangoapps.certificates.models import CertificateWhitelist, GeneratedCertificate, certificate_info_for_user +from lms.djangoapps.certificates import api as certs_api +from lms.djangoapps.certificates.models import GeneratedCertificate, certificate_info_for_user from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.grades.api import CourseGradeFactory @@ -358,8 +359,8 @@ class _ProblemGradeReportContext: class _CertificateBulkContext: def __init__(self, context, users): - certificate_whitelist = CertificateWhitelist.objects.filter(course_id=context.course_id, whitelist=True) - self.whitelisted_user_ids = [entry.user_id for entry in certificate_whitelist] + certificate_allowlist = certs_api.get_allowlist(context.course_id) + self.allowlisted_user_ids = [entry['user_id'] for entry in certificate_allowlist] self.certificates_by_user = { certificate.user.id: certificate for certificate in @@ -661,12 +662,12 @@ class CourseGradeReport: """ Returns the course certification information for the given user. """ - is_whitelisted = user.id in bulk_certs.whitelisted_user_ids + is_allowlisted = user.id in bulk_certs.allowlisted_user_ids certificate_info = certificate_info_for_user( user, context.course_id, course_grade.letter_grade, - is_whitelisted, + is_allowlisted, bulk_certs.certificates_by_user.get(user.id), ) return certificate_info diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index ce8cfabc3b..8d3ffb08ba 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1910,7 +1910,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas def _create_user_data(self, user_enroll_mode, has_passed, - whitelisted, verification_status, certificate_status, certificate_mode): @@ -1923,7 +1922,7 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas if has_passed: self.submit_student_answer('u1', 'test_problem', ['choice_1']) - CertificateAllowlistFactory.create(user=user, course_id=self.course.id, whitelist=whitelisted) + CertificateAllowlistFactory.create(user=user, course_id=self.course.id) if user_enroll_mode in CourseMode.VERIFIED_MODES: SoftwareSecurePhotoVerificationFactory.create(user=user, status=verification_status) @@ -1939,19 +1938,19 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas @ddt.data( ( - 'verified', False, False, 'approved', 'notpassing', 'honor', - ['verified', 'ID Verified', 'N', 'N', 'N/A'] + 'verified', False, 'approved', 'notpassing', 'honor', + ['verified', 'ID Verified', 'Y', 'N', 'N/A'] ), ( - 'verified', False, True, 'approved', 'downloadable', 'verified', + 'verified', False, 'approved', 'downloadable', 'verified', ['verified', 'ID Verified', 'Y', 'Y', 'verified'] ), ( - 'honor', True, True, 'approved', 'restricted', 'honor', + 'honor', True, 'approved', 'restricted', 'honor', ['honor', 'N/A', 'Y', 'N', 'N/A'] ), ( - 'verified', True, True, 'must_retry', 'downloadable', 'honor', + 'verified', True, 'must_retry', 'downloadable', 'honor', ['verified', 'Not ID Verified', 'Y', 'Y', 'honor'] ), ) @@ -1960,7 +1959,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas self, user_enroll_mode, has_passed, - whitelisted, verification_status, certificate_status, certificate_mode, @@ -1970,7 +1968,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas user = self._create_user_data( user_enroll_mode, has_passed, - whitelisted, verification_status, certificate_status, certificate_mode @@ -2010,7 +2007,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Allowlist 5 students for student in students[2:7]: - CertificateAllowlistFactory.create(user=student, course_id=self.course.id, whitelist=True) + CertificateAllowlistFactory.create(user=student, course_id=self.course.id,) task_input = {'student_set': None} expected_results = { @@ -2041,7 +2038,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Allowlist 3 students for student in students[:3]: CertificateAllowlistFactory.create( - user=student, course_id=self.course.id, whitelist=True + user=student, course_id=self.course.id ) # Grant certs to 2 students @@ -2091,7 +2088,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Allowlist 4 students for student in students[:4]: CertificateAllowlistFactory.create( - user=student, course_id=self.course.id, whitelist=True + user=student, course_id=self.course.id ) task_input = {'student_set': 'whitelisted_not_generated'} @@ -2116,7 +2113,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): Tests generating a certificate for a specific student. """ student = self.create_student(username="Hamnet", email="ham@ardenforest.co.uk") - CertificateAllowlistFactory.create(user=student, course_id=self.course.id, whitelist=True) + CertificateAllowlistFactory.create(user=student, course_id=self.course.id) task_input = { 'student_set': 'specific_student', 'specific_student_id': student.id @@ -2188,7 +2185,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Allowlist 7 students for student in students[:7]: - CertificateAllowlistFactory.create(user=student, course_id=self.course.id, whitelist=True) + CertificateAllowlistFactory.create(user=student, course_id=self.course.id) # Certificates should be regenerated for students having generated certificates with status # 'downloadable' or 'error' which are total of 5 students in this test case @@ -2261,7 +2258,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Allowlist 7 students for student in students[:7]: - CertificateAllowlistFactory.create(user=student, course_id=self.course.id, whitelist=True) + CertificateAllowlistFactory.create(user=student, course_id=self.course.id) # Regenerated certificates for students having generated certificates with status # 'deleted' or 'generating' @@ -2332,7 +2329,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Allowlist all students for student in students[:]: - CertificateAllowlistFactory.create(user=student, course_id=self.course.id, whitelist=True) + CertificateAllowlistFactory.create(user=student, course_id=self.course.id) # Regenerated certificates for students having generated certificates with status # 'downloadable', 'error' or 'generating' @@ -2403,7 +2400,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): # Allowlist 7 students for student in students[:7]: - CertificateAllowlistFactory.create(user=student, course_id=self.course.id, whitelist=True) + CertificateAllowlistFactory.create(user=student, course_id=self.course.id) # Certificates should be regenerated for students having generated certificates with status # 'downloadable' or 'error' which are total of 5 students in this test case