diff --git a/lms/djangoapps/certificates/docs/decisions/001-allowlist-cert-requirements.rst b/lms/djangoapps/certificates/docs/decisions/001-allowlist-cert-requirements.rst index 59cb951406..7d7c20253f 100644 --- a/lms/djangoapps/certificates/docs/decisions/001-allowlist-cert-requirements.rst +++ b/lms/djangoapps/certificates/docs/decisions/001-allowlist-cert-requirements.rst @@ -33,7 +33,3 @@ the time the certificate is generated: * The user must not have an invalidated certificate for the course run (see the *CertificateInvalidation* model) * HTML (web) certificates must be globally enabled, and also enabled for the course run * The user must be on the allowlist for the course run (see the *CertificateWhitelist* model) - -Note: the above requirements were written for the allowlist, which assumes the -CourseWaffleFlag *certificates_revamp.use_allowlist* has been enabled for the -course run. If it has not been enabled, the prior logic will apply. diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index eaad62b938..63ab1f045b 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -373,8 +373,12 @@ def is_using_certificate_allowlist_and_is_on_allowlist(user, course_key): def is_using_certificate_allowlist(course_key): """ Check if the course run is using the allowlist, aka v2 of certificate whitelisting + + Currently this returns True for all course runs, as we're enabling the allowlist for everyone. + If all goes well, we'll come back and remove this method and the flag entirely in MICROBA-1073. """ - return CERTIFICATES_USE_ALLOWLIST.is_enabled(course_key) + return True +# return CERTIFICATES_USE_ALLOWLIST.is_enabled(course_key) def is_using_v2_course_certificates(course_key): diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 5bebdd2b9b..892c9d09c3 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -56,7 +56,6 @@ from lms.djangoapps.certificates.api import ( remove_allowlist_entry, set_cert_generation_enabled ) -from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_ALLOWLIST from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, CertificateStatuses, @@ -802,7 +801,6 @@ class CertificatesBrandingTest(ModuleStoreTestCase): assert self.configuration['urls']['TOS_AND_HONOR'] in data['company_tos_url'] -@override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) class AllowlistTests(ModuleStoreTestCase): """ Tests for handling allowlist certificates @@ -871,20 +869,6 @@ class AllowlistTests(ModuleStoreTestCase): users = get_allowlisted_users(self.third_course_run_key) assert 0 == users.count() - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=False) - def test_get_users_allowlist_false(self): - """ - Test - """ - users = get_allowlisted_users(self.course_run_key) - assert 0 == users.count() - - users = get_allowlisted_users(self.second_course_run_key) - assert 0 == users.count() - - users = get_allowlisted_users(self.third_course_run_key) - assert 0 == users.count() - class CertificateAllowlistTests(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index 5d101a6f02..371e4cc158 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -9,7 +9,6 @@ from edx_toggles.toggles.testutils import override_waffle_flag from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.certificates.generation_handler import ( - CERTIFICATES_USE_ALLOWLIST, CERTIFICATES_USE_UPDATED, _can_generate_allowlist_certificate, _can_generate_certificate_for_status, @@ -42,7 +41,6 @@ PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._has_pass WEB_CERTS_METHOD = 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled' -@override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) @mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True)) @mock.patch(WEB_CERTS_METHOD, mock.Mock(return_value=True)) @ddt.ddt @@ -74,26 +72,12 @@ class AllowlistTests(ModuleStoreTestCase): """ assert is_using_certificate_allowlist(self.course_run_key) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=False) - def test_is_using_allowlist_false(self): - """ - Test the allowlist flag without the override - """ - assert not is_using_certificate_allowlist(self.course_run_key) - def test_is_using_allowlist_and_is_on_list(self): """ Test the allowlist flag and the presence of the user on the list """ assert is_using_certificate_allowlist_and_is_on_allowlist(self.user, self.course_run_key) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=False) - def test_is_using_allowlist_and_is_on_list_with_flag_off(self): - """ - Test the allowlist flag and the presence of the user on the list when the flag is off - """ - assert not is_using_certificate_allowlist_and_is_on_allowlist(self.user, self.course_run_key) - def test_is_using_allowlist_and_is_on_list_true(self): """ Test the allowlist flag and the presence of the user on the list when the user is not on the list @@ -146,7 +130,6 @@ class AllowlistTests(ModuleStoreTestCase): """ assert _can_generate_certificate_for_status(None, None) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=False) def test_handle_invalid(self): """ Test handling of an invalid user/course run combo diff --git a/lms/djangoapps/certificates/tests/test_services.py b/lms/djangoapps/certificates/tests/test_services.py index b422f8dea5..447d96fefc 100644 --- a/lms/djangoapps/certificates/tests/test_services.py +++ b/lms/djangoapps/certificates/tests/test_services.py @@ -5,7 +5,6 @@ Unit Tests for the Certificate service from edx_toggles.toggles.testutils import override_waffle_flag from common.djangoapps.student.tests.factories import UserFactory -from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_ALLOWLIST from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.services import CertificateService from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory, GeneratedCertificateFactory @@ -66,7 +65,6 @@ class CertificateServiceTests(ModuleStoreTestCase): } ) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) def test_invalidate_certificate_allowlist(self): """ Verify that CertificateService does not invalidate the certificate if it is allowlisted diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index c249742905..d17951ac71 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -12,10 +12,7 @@ from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.certificates.api import cert_generation_enabled -from lms.djangoapps.certificates.generation_handler import ( - CERTIFICATES_USE_ALLOWLIST, - CERTIFICATES_USE_UPDATED -) +from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_UPDATED from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, CertificateStatuses, @@ -78,7 +75,6 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): mode="verified", ) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) def test_fire_task_allowlist_auto_enabled(self): """ Test that the allowlist generation is invoked if automatic generation is enabled @@ -100,7 +96,6 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): mock_generate_certificate_apply_async.assert_not_called() mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) def test_fire_task_allowlist_auto_disabled(self): """ Test that the allowlist generation is not invoked if automatic generation is disabled @@ -122,33 +117,6 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase): mock_generate_certificate_apply_async.assert_not_called() mock_generate_allowlist_task.assert_not_called() - def test_fire_task_allowlist_disabled(self): - """ - Test that the V1 logic is followed if the allowlist is disabled - """ - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', - return_value=None - ) as mock_generate_certificate_apply_async: - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task', - return_value=None - ) as mock_generate_allowlist_task: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): - CertificateWhitelistFactory( - user=self.user, - course_id=self.ip_course.id, - whitelist=True - ) - mock_generate_certificate_apply_async.assert_called_with( - countdown=CERTIFICATE_DELAY_SECONDS, - kwargs={ - 'student': str(self.user.id), - 'course_key': str(self.ip_course.id), - } - ) - mock_generate_allowlist_task.assert_not_called() - class PassingGradeCertsTest(ModuleStoreTestCase): """ @@ -240,7 +208,6 @@ class PassingGradeCertsTest(ModuleStoreTestCase): grade_factory.update(self.user, self.course) mock_generate_certificate_apply_async.assert_not_called() - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) def test_passing_grade_allowlist(self): with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): # User who is not on the allowlist @@ -383,7 +350,6 @@ class FailingGradeCertsTest(ModuleStoreTestCase): cert = GeneratedCertificate.certificate_for_student(self.user, self.course.id) assert cert.status == expected_status - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) def test_failing_grade_allowlist(self): # User who is not on the allowlist GeneratedCertificateFactory( @@ -483,7 +449,6 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase): } ) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) def test_id_verification_allowlist(self): # User is not on the allowlist with mock.patch( @@ -567,7 +532,6 @@ class CertificateGenerationTaskTest(ModuleStoreTestCase): assert task_created == should_create -@override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) @override_waffle_flag(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True) class EnrollmentModeChangeCertsTest(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 33c9c5b17d..ffdcf6274d 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1526,7 +1526,7 @@ class ProgressPageTests(ProgressPageBaseTests): @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 61, 44), + (False, 62, 45), (True, 54, 39) ) @ddt.unpack diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 64939cbb35..9a95f82746 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -119,7 +119,7 @@ class TestCourseGradeFactory(GradeTestBase): with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 30 + num_queries = 29 with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero grade_factory.update(self.request.user, self.course, force_update_subsections=True) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 4b64f4d1fa..c68b17f37b 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -162,10 +162,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest assert mock_block_structure_create.call_count == 1 @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 40, True), - (ModuleStoreEnum.Type.mongo, 1, 40, False), - (ModuleStoreEnum.Type.split, 3, 40, True), - (ModuleStoreEnum.Type.split, 3, 40, False), + (ModuleStoreEnum.Type.mongo, 1, 39, True), + (ModuleStoreEnum.Type.mongo, 1, 39, False), + (ModuleStoreEnum.Type.split, 3, 39, True), + (ModuleStoreEnum.Type.split, 3, 39, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -177,8 +177,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 40), - (ModuleStoreEnum.Type.split, 3, 40), + (ModuleStoreEnum.Type.mongo, 1, 39), + (ModuleStoreEnum.Type.split, 3, 39), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -223,8 +223,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 23), - (ModuleStoreEnum.Type.split, 3, 23), + (ModuleStoreEnum.Type.mongo, 1, 22), + (ModuleStoreEnum.Type.split, 3, 22), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -238,8 +238,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest assert len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)) == 0 @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 41), - (ModuleStoreEnum.Type.split, 3, 41), + (ModuleStoreEnum.Type.mongo, 1, 40), + (ModuleStoreEnum.Type.split, 3, 40), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 728c5e2c75..08766037a1 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -30,10 +30,7 @@ from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from lms.djangoapps.certificates.generation_handler import ( - CERTIFICATES_USE_ALLOWLIST, - CERTIFICATES_USE_UPDATED -) +from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_UPDATED from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory, GeneratedCertificateFactory from lms.djangoapps.courseware.models import StudentModule @@ -2024,7 +2021,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 0, 'skipped': 2 } - with self.assertNumQueries(170): + with self.assertNumQueries(114): self.assertCertificatesGenerated(task_input, expected_results) @ddt.data( @@ -2423,7 +2420,6 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): self.assertCertificatesGenerated(task_input, expected_results) - @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True) def test_invalidation(self): # Create students students = self._create_students(2)