From ce591fd3eabc12a2a0db556470e720be0a4a1b7c Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Wed, 5 May 2021 10:51:31 -0400 Subject: [PATCH] refactor: Rename whitelist to allowlist (#27504) MICROBA-1021 --- lms/djangoapps/certificates/api.py | 7 ++ .../instructor/tests/test_certificates.py | 71 ++++++++++--------- lms/djangoapps/instructor/views/api.py | 34 ++++----- .../instructor/views/instructor_dashboard.py | 5 +- 4 files changed, 63 insertions(+), 54 deletions(-) diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index ea9e337e20..85b649ea0a 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -713,3 +713,10 @@ def is_using_v2_course_certificates(course_key): Determines if the given course run is using V2 of course certificates """ return _is_using_v2_course_certificates(course_key) + + +def get_allowlist(course_key): + """ + Return the certificate allowlist for the given course run + """ + return CertificateWhitelist.get_certificate_white_list(course_key) diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index 65f519e65d..fdce5e4359 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -28,7 +28,6 @@ from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, CertificateInvalidation, CertificateStatuses, - CertificateWhitelist, GeneratedCertificate ) from lms.djangoapps.certificates.tests.factories import ( @@ -495,7 +494,7 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase): CourseEnrollment.enroll(self.user2, self.course.id) self.url = reverse('certificate_exception_view', kwargs={'course_id': str(self.course.id)}) - certificate_white_list_item = CertificateAllowlistFactory.create( + certificate_allowlist_item = CertificateAllowlistFactory.create( user=self.user2, course_id=self.course.id, ) @@ -509,11 +508,11 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase): ) self.certificate_exception_in_db = dict( - id=certificate_white_list_item.id, - user_name=certificate_white_list_item.user.username, - notes=certificate_white_list_item.notes, - user_email=certificate_white_list_item.user.email, - user_id=certificate_white_list_item.user.id, + id=certificate_allowlist_item.id, + user_name=certificate_allowlist_item.user.username, + notes=certificate_allowlist_item.notes, + user_email=certificate_allowlist_item.user.email, + user_id=certificate_allowlist_item.user.id, ) # Enable certificate generation @@ -589,8 +588,8 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase): def test_certificate_exception_duplicate_user_error(self): """ - Test certificates exception addition api endpoint returns failure when called with - username/email that already exists in 'CertificateWhitelist' table. + Ensure the certificates exception endpoint returns failure when called with + username/email that already exists on the certificate allowlist. """ response = self.client.post( self.url, @@ -684,6 +683,9 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase): status=CertificateStatuses.downloadable, grade='1.0' ) + # Verify that certificate exception exists + assert certs_api.is_on_allowlist(self.user2, self.course.id) + response = self.client.post( self.url, data=json.dumps(self.certificate_exception_in_db), @@ -693,12 +695,8 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase): # Assert successful request processing assert response.status_code == 204 - # Verify that certificate exception successfully removed from CertificateWhitelist and GeneratedCertificate - with pytest.raises(ObjectDoesNotExist): - CertificateWhitelist.objects.get(user=self.user2, course_id=self.course.id) - GeneratedCertificate.eligible_certificates.get( - user=self.user2, course_id=self.course.id, status__not=CertificateStatuses.unavailable - ) + # Verify that certificate exception does not exist + assert not certs_api.is_on_allowlist(self.user2, self.course.id) def test_remove_certificate_exception_invalid_request_error(self): """ @@ -839,7 +837,7 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): # Assert Message assert res_json['message'] == 'Certificate generation started for white listed students.' - def test_generate_certificate_exceptions_whitelist_not_generated(self): + def test_generate_certificate_exceptions_allowlist_not_generated(self): """ Test generate certificates exceptions api endpoint returns success when calling with new certificate exception. @@ -891,9 +889,9 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): @ddt.ddt -class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTestCase): +class TestCertificatesInstructorApiBulkAllowlist(SharedModuleStoreTestCase): """ - Test Bulk certificates white list exceptions from csv file + Test bulk certificates allowlist uploads from csv file """ @classmethod def setUpClass(cls): @@ -905,22 +903,25 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest def setUp(self): super().setUp() self.global_staff = GlobalStaffFactory() + self.enrolled_user_1_email = 'test_student1@example.com' self.enrolled_user_1 = UserFactory( username='TestStudent1', - email='test_student1@example.com', + email=self.enrolled_user_1_email, first_name='Enrolled', last_name='Student' ) + self.enrolled_user_2_email = 'test_student2@example.com' self.enrolled_user_2 = UserFactory( username='TestStudent2', - email='test_student2@example.com', + email=self.enrolled_user_2_email, first_name='Enrolled', last_name='Student' ) + self.not_enrolled_user_email = 'nonenrolled@test.com' self.not_enrolled_student = UserFactory( username='NotEnrolledStudent', - email='nonenrolled@test.com', + email=self.not_enrolled_user_email, first_name='NotEnrolled', last_name='Student' ) @@ -930,10 +931,13 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest # Global staff can see the certificates section self.client.login(username=self.global_staff.username, password="test") - def test_create_white_list_exception_record(self): + def test_create_allowlist_exception_record(self): """ - Happy path test to create a single new white listed record + Happy path test to create a single new allowlisted record """ + assert not certs_api.is_on_allowlist(self.enrolled_user_1, self.course.id) + assert not certs_api.is_on_allowlist(self.enrolled_user_2, self.course.id) + csv_content = b"test_student1@example.com,dummy_notes\n" \ b"test_student2@example.com,dummy_notes" data = self.upload_file(csv_content=csv_content) @@ -943,7 +947,9 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest assert len(data['row_errors']['user_already_white_listed']) == 0 assert len(data['row_errors']['user_not_enrolled']) == 0 assert len(data['success']) == 2 - assert len(CertificateWhitelist.objects.all()) == 2 + + assert certs_api.is_on_allowlist(self.enrolled_user_1, self.course.id) + assert certs_api.is_on_allowlist(self.enrolled_user_2, self.course.id) def test_invalid_data_format_in_csv(self): """ @@ -956,7 +962,9 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest assert len(data['row_errors']['data_format_error']) == 2 assert len(data['general_errors']) == 0 assert len(data['success']) == 0 - assert len(CertificateWhitelist.objects.all()) == 0 + + assert not certs_api.is_on_allowlist(self.enrolled_user_1, self.course.id) + assert not certs_api.is_on_allowlist(self.enrolled_user_2, self.course.id) def test_file_upload_type_not_csv(self): """ @@ -990,7 +998,6 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest data = self.upload_file(csv_content=csv_content) assert len(data['row_errors']['user_not_exist']) == 1 assert len(data['success']) == 0 - assert len(CertificateWhitelist.objects.all()) == 0 def test_csv_user_not_enrolled(self): """ @@ -1001,24 +1008,20 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest data = self.upload_file(csv_content=csv_content) assert len(data['row_errors']['user_not_enrolled']) == 1 assert len(data['general_errors']) == 0 - assert len(data['success']) == 0 + + assert not certs_api.is_on_allowlist(self.not_enrolled_student, self.course.id) def test_certificate_exception_already_exist(self): """ Test error if existing user is already in certificates exception list. """ - CertificateWhitelist.objects.create( - user=self.enrolled_user_1, - course_id=self.course.id, - whitelist=True, - notes='' - ) + CertificateAllowlistFactory.create(user=self.enrolled_user_1, course_id=self.course.id) + csv_content = b"test_student1@example.com,dummy_notes" data = self.upload_file(csv_content=csv_content) assert len(data['row_errors']['user_already_white_listed']) == 1 assert len(data['general_errors']) == 0 assert len(data['success']) == 0 - assert len(CertificateWhitelist.objects.all()) == 1 def test_csv_file_not_attached(self): """ diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 66d7b6ed3c..0cb63769d3 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -656,7 +656,7 @@ def students_update_enrollment(request, course_id): # lint-amnesty, pylint: dis results = [] for identifier in identifiers: # lint-amnesty, pylint: disable=too-many-nested-blocks - # First try to get a user object from the identifer + # First try to get a user object from the identifier user = None email = None language = None @@ -855,7 +855,7 @@ def modify_access(request, course_id): NOTE: instructors cannot remove their own instructor access. Query parameters: - unique_student_identifer is the target user's username or email + unique_student_identifier is the target user's username or email rolename is one of ['instructor', 'staff', 'beta', 'ccx_coach'] action is one of ['allow', 'revoke'] """ @@ -1474,7 +1474,7 @@ def reset_student_attempts(request, course_id): attempts counters. Optionally deletes student state for a problem. Limited to staff access. Some sub-methods limited to instructor access. - Takes some of the following query paremeters + Takes some of the following query parameters - problem_to_reset is a urlname of a problem - unique_student_identifier is an email or username - all_students is a boolean @@ -1632,7 +1632,7 @@ def rescore_problem(request, course_id): Starts a background process a students attempts counter. Optionally deletes student state for a problem. Rescore for all students is limited to instructor access. - Takes either of the following query paremeters + Takes either of the following query parameters - problem_to_reset is a urlname of a problem - unique_student_identifier is an email or username - all_students is a boolean @@ -1860,7 +1860,7 @@ def list_instructor_tasks(request, course_id): """ List instructor tasks. - Takes optional query paremeters. + Takes optional query parameters. - With no arguments, lists running tasks. - `problem_location_str` lists task history for problem - `problem_location_str` and `unique_student_identifier` lists task @@ -2455,7 +2455,7 @@ def generate_example_certificates(request, course_id=None): Example certificates are used to verify that certificates have been configured correctly for the course. - Redirects back to the intructor dashboard once certificate + Redirects back to the instructor dashboard once certificate generation has begun. """ @@ -2472,7 +2472,7 @@ def enable_certificate_generation(request, course_id=None): Once self-generated certificates have been enabled, students who have passed the course will be able to generate certificates. - Redirects back to the intructor dashboard once the + Redirects back to the instructor dashboard once the setting has been updated. """ @@ -2577,7 +2577,7 @@ def start_certificate_regeneration(request, course_id): @require_http_methods(['POST', 'DELETE']) def certificate_exception_view(request, course_id): """ - Add/Remove students to/from certificate white list. + Add/Remove students to/from the certificate allowlist. :param request: HttpRequest object :param course_id: course identifier of the course for whom to add/remove certificates exception. @@ -2610,14 +2610,14 @@ def certificate_exception_view(request, course_id): def add_certificate_exception(course_key, student, certificate_exception): """ - Add a certificate exception to CertificateWhitelist table. + Add a certificate exception. - Raises ValueError in case Student is already white listed or if they appear on the block list. + Raises ValueError in case Student is already allowlisted or if they appear on the block list. :param course_key: identifier of the course whose certificate exception will be added. :param student: User object whose certificate exception will be added. :param certificate_exception: A dict object containing certificate exception info. - :return: CertificateWhitelist item in dict format containing certificate exception info. + :return: Allowlist item in dict format containing certificate exception info. """ log.info(f"Request received to add an allowlist entry for student {student.id} in course {course_key}") @@ -2740,18 +2740,18 @@ def get_student(username_or_email): @common_exceptions_400 def generate_certificate_exceptions(request, course_id, generate_for=None): """ - Generate Certificate for students in the Certificate White List. + Generate Certificate for students on the allowlist. :param request: HttpRequest object, :param course_id: course identifier of the course for whom to generate certificates :param generate_for: string to identify whether to generate certificates for 'all' or 'new' - additions to the certificate white-list + additions to the allowlist :return: JsonResponse object containing success/failure message and certificate exception data """ course_key = CourseKey.from_string(course_id) if generate_for == 'all': - # Generate Certificates for all white listed students + # Generate Certificates for all allowlisted students students = 'all_whitelisted' elif generate_for == 'new': @@ -2781,18 +2781,18 @@ def generate_certificate_exceptions(request, course_id, generate_for=None): @require_POST def generate_bulk_certificate_exceptions(request, course_id): """ - Add Students to certificate white list from the uploaded csv file. + Add Students to certificate allowlist from the uploaded csv file. :return response in dict format. { general_errors: [errors related to csv file e.g. csv uploading, csv attachment, content reading etc. ], row_errors: { data_format_error: [users/data in csv file that are not well formatted], user_not_exist: [csv with none exiting users in LMS system], - user_already_white_listed: [users that are already white listed], + user_already_white_listed: [users that are already allowlisted], user_not_enrolled: [rows with not enrolled users in the given course], user_on_certificate_invalidation_list: [users that are currently have an active blocklist entry] }, - success: [list of successfully added users to the certificate white list model] + success: [list of successfully added users to the certificate allowlist model] } """ user_index = 0 diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index dbdb31e401..65a2c8f5f0 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -44,7 +44,6 @@ from lms.djangoapps.certificates.models import ( CertificateGenerationHistory, CertificateInvalidation, CertificateStatuses, - CertificateWhitelist, GeneratedCertificate ) from lms.djangoapps.courseware.access import has_access @@ -212,7 +211,7 @@ def instructor_dashboard_2(request, course_id): # lint-amnesty, pylint: disable disable_buttons = not CourseEnrollment.objects.is_small_course(course_key) - certificate_white_list = CertificateWhitelist.get_certificate_white_list(course_key) + certificate_allowlist = certs_api.get_allowlist(course_key) generate_certificate_exceptions_url = reverse( 'generate_certificate_exceptions', kwargs={'course_id': str(course_key), 'generate_for': ''} @@ -239,7 +238,7 @@ def instructor_dashboard_2(request, course_id): # lint-amnesty, pylint: disable 'sections': sections, 'disable_buttons': disable_buttons, 'analytics_dashboard_message': analytics_dashboard_message, - 'certificate_white_list': certificate_white_list, + 'certificate_white_list': certificate_allowlist, 'certificate_invalidations': certificate_invalidations, 'generate_certificate_exceptions_url': generate_certificate_exceptions_url, 'generate_bulk_certificate_exceptions_url': generate_bulk_certificate_exceptions_url,