refactor: Rename whitelist to allowlist (#27533)

MICROBA-1021
2021-05-12 09:51:26 -04:00
parent c651e58fb7
commit 1473973b2b
6 changed files with 66 additions and 48 deletions
--- 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
+    )
--- 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()
--- 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:
--- 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')
--- 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
--- 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