From 2980265764b30c7636416b5d71eca60b5d22a47d Mon Sep 17 00:00:00 2001 From: Mubbshar Anwar <78487564+mubbsharanwar@users.noreply.github.com> Date: Tue, 30 Aug 2022 12:11:25 +0500 Subject: [PATCH] Revert "feat: Pass segment properties (#30742)" (#30918) This reverts commit b76d5c2a66aa662a44cda5277ac6abeefec9358f. --- common/djangoapps/student/helpers.py | 93 +--------- .../tests/test_transfer_students.py | 10 +- common/djangoapps/student/models.py | 46 +---- .../student/tests/test_enrollment.py | 12 -- common/djangoapps/student/tests/tests.py | 163 ++---------------- .../tests/test_field_override_performance.py | 5 - .../outline/tests/test_view.py | 4 - .../progress/tests/test_views.py | 5 - lms/djangoapps/courseware/tests/test_views.py | 18 +- .../grades/tests/test_course_grade_factory.py | 10 -- .../tests/test_tasks_helper.py | 4 +- .../djangoapps/credit/tests/test_services.py | 4 +- .../enrollments/tests/test_views.py | 41 ----- .../schedules/tests/test_resolvers.py | 36 +--- .../content_type_gating/tests/test_access.py | 5 +- 15 files changed, 21 insertions(+), 435 deletions(-) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index fa966874b7..71b46d5af5 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -20,7 +20,7 @@ from django.core.validators import ValidationError from django.db import IntegrityError, ProgrammingError, transaction from django.urls import NoReverseMatch, reverse from django.utils.translation import gettext as _ -from pytz import UTC, timezone +from pytz import UTC from common.djangoapps import third_party_auth from common.djangoapps.course_modes.models import CourseMode @@ -50,15 +50,10 @@ from lms.djangoapps.instructor import access from lms.djangoapps.verify_student.models import VerificationDeadline from lms.djangoapps.verify_student.services import IDVerificationService from lms.djangoapps.verify_student.utils import is_verification_expiring_soon, verification_for_datetime -from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course_with_access -from lms.djangoapps.courseware.date_summary import TodaysDate -from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs -from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer from openedx.core.djangoapps.content.block_structure.exceptions import UsageKeyNotInBlockStructure from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming.helpers import get_themes from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect -from openedx.core.lib.time_zone_utils import get_time_zone_offset from xmodule.data import CertificatesDisplayBehaviors # lint-amnesty, pylint: disable=wrong-import-order # Enumeration of per-course verification statuses @@ -819,89 +814,3 @@ def user_has_passing_grade_in_course(enrollment): except AttributeError: pass return False - - -def get_instructors(course_run, marketing_root_url): - """ - Get course instructors. - """ - instructors = [] - staff = course_run.get('staff', []) - for instructor in staff: - instructor = { - 'name': f"{instructor.get('given_name')} {instructor.get('family_name')}", - 'profile_image_url': instructor.get('profile_image_url'), - 'organization_name': (instructor.get('position').get('organization_name') - if instructor.get('position') else ''), - 'bio_url': f"{marketing_root_url}/bio/{instructor.get('slug')}" - } - instructors.append(instructor) - - return instructors - - -def _prepare_date_block(block, block_date, user_timezone): - """ - Prepare date block which include assignment related data for this date - """ - timezone_offset = get_time_zone_offset(user_timezone, block_date) - block = { - 'title': block.get('title', ''), - 'assignment_type': block.get('assignment_type', ''), - 'assignment_count': 0, - 'link': block.get('link', ''), - 'date': block_date, - 'due_date': block_date.strftime("%a, %b %d, %Y"), - 'due_time': (f'{block_date.strftime("%H:%M %p")} GMT{timezone_offset}' if block.get('assignment_type') else '') - } - return block - - -def get_course_dates_for_email(user, course_id, request): - """ - Getting nearest dates from today one would be before today and one - would be after today. - """ - user_timezone_locale = user_timezone_locale_prefs(request) - user_timezone = timezone(user_timezone_locale['user_timezone'] or str(UTC)) - - course = get_course_with_access(user, 'load', course_id) - date_blocks = get_course_date_blocks(course, user, request, include_access=True, include_past_dates=True) - date_blocks = [block for block in date_blocks if not isinstance(block, TodaysDate)] - blocks = DateSummarySerializer(date_blocks, many=True).data - - today = datetime.now(user_timezone) - course_date = { - 'title': '', - 'assignment_type': '', - 'link': '', - 'assignment_count': 0, - 'date': '', - 'due_date': today.strftime("%a, %b %d, %Y"), - 'due_time': '' - } - course_date_list = [{**course_date, }, {**course_date, 'date': today}, {**course_date}] - for block in blocks: - block_date = datetime.strptime(block.get('date')[:19], '%Y-%m-%dT%H:%M:%S') - block_date = block_date.replace(tzinfo=UTC) - block_date = block_date.astimezone(user_timezone) - - if block_date < today: - if block_date == course_date_list[0]['date'] and block.get('assignment_type'): - course_date_list[0]['assignment_count'] += 1 - else: - course_date_list[0].update(_prepare_date_block(block, block_date, user_timezone)) - - if block_date == today: - if block.get('assignment_type') and course_date_list[1]['assignment_type'] != '': - course_date_list[1]['assignment_count'] += 1 - else: - course_date_list[1].update(_prepare_date_block(block, block_date, user_timezone)) - - if block_date > today: - if block_date == course_date_list[2]['date'] and block.get('assignment_type'): - course_date_list[2]['assignment_count'] += 1 - if course_date_list[2]['date'] == '': - course_date_list[2].update(_prepare_date_block(block, block_date, user_timezone)) - - return course_date_list diff --git a/common/djangoapps/student/management/tests/test_transfer_students.py b/common/djangoapps/student/management/tests/test_transfer_students.py index f9ee5c808a..f42ab66e10 100644 --- a/common/djangoapps/student/management/tests/test_transfer_students.py +++ b/common/djangoapps/student/management/tests/test_transfer_students.py @@ -20,7 +20,6 @@ from common.djangoapps.student.models import ( ) from common.djangoapps.student.signals import UNENROLL_DONE from common.djangoapps.student.tests.factories import UserFactory -from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -55,8 +54,7 @@ class TestTransferStudents(ModuleStoreTestCase): assert skip_refund self.signal_fired = True - @patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - def test_transfer_students(self, mock_get_course_run_details): + def test_transfer_students(self): """ Verify the transfer student command works as intended. """ @@ -67,12 +65,6 @@ class TestTransferStudents(ModuleStoreTestCase): # Original Course original_course_location = locator.CourseLocator('Org0', 'Course0', 'Run0') course = self._create_course(original_course_location) - - course_run = CourseRunFactory.create(key=course.id) - course_run['min_effort'] = 1 - course_run['enrollment_count'] = 12345 - - mock_get_course_run_details.return_value = course_run # Enroll the student in 'verified' CourseEnrollment.enroll(student, course.id, mode='verified') diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index c873cf38ac..2219afb572 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1554,15 +1554,11 @@ class CourseEnrollment(models.Model): mode=mode, course_id=course_id, cost=cost, currency=currency) - def emit_event(self, event_name, enterprise_uuid=None): # pylint: disable=too-many-statements + def emit_event(self, event_name, enterprise_uuid=None): """ Emits an event to explicitly track course enrollment and unenrollment. """ - from common.djangoapps.student.helpers import get_course_dates_for_email, get_instructors from openedx.core.djangoapps.schedules.config import set_up_external_updates_for_enrollment - from openedx.core.djangoapps.catalog.api import get_course_run_details - from openedx.core.djangoapps.catalog.utils import get_owners_for_course, get_course_uuid_for_course - from openedx.features.course_experience import ENABLE_COURSE_GOALS from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from openedx.features.enterprise_support.utils import is_enterprise_learner @@ -1600,46 +1596,6 @@ class CourseEnrollment(models.Model): segment_traits['email'] = self.user.email if event_name == EVENT_NAME_ENROLLMENT_ACTIVATED: - request = crum.get_current_request() - marketing_root_url = settings.MKTG_URLS.get('ROOT') - course_date_list = get_course_dates_for_email(self.user, self.course.id, request) - - course_run_fields = [ - 'key', 'title', 'short_description', 'marketing_url', 'pacing_type', 'min_effort', - 'max_effort', 'weeks_to_complete', 'enrollment_count', 'image', 'staff', - ] - course_run = get_course_run_details(str(self.course_id), course_run_fields) - - course_uuid = get_course_uuid_for_course(str(self.course_id)) - owners = get_owners_for_course(course_uuid=course_uuid) - extra_segment_properties = {} - if course_run: - instructors = get_instructors(course_run, marketing_root_url) - extra_segment_properties.update({ - 'instructors': instructors, - 'instructors_count': 'even' if len(instructors) % 2 == 0 else 'odd', - 'pacing_type': course_run.get('pacing_type'), - 'min_effort': course_run.get('min_effort'), - 'max_effort': course_run.get('max_effort'), - 'weeks_to_complete': course_run.get('weeks_to_complete'), - 'learners_count': '{:,}'.format(course_run.get('enrollment_count')), - 'course_title': course_run.get('title'), - 'short_description': course_run.get('short_description'), - 'marketing_url': course_run.get('marketing_url'), - 'banner_image_url': course_run.get('image').get('src') if course_run.get('image') else '' - }) - extra_segment_properties.update({ - 'goals_enabled': ENABLE_COURSE_GOALS.is_enabled(self.course_id), - 'course_date_blocks': course_date_list, - 'partner_image_url': owners[0].get('logo_image_url') if owners else '', - 'price': self.course_price, - 'learner_name': self.user.profile.name, - 'course_run_key': str(self.course_id), - 'lms_base_url': configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), - 'learning_base_url': (configuration_helpers. - get_value('LEARNING_MICROFRONTEND_URL', settings.LEARNING_MICROFRONTEND_URL)) - }) - segment_properties.update(extra_segment_properties) segment_properties['email'] = self.user.email # This next property is for an experiment, see method's comments for more information segment_properties['external_course_updates'] = set_up_external_updates_for_enrollment(self.user, diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index 5e332bc2e5..520f5a594e 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -26,7 +26,6 @@ from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRol from common.djangoapps.student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory from common.djangoapps.util.testing import UrlResetMixin from openedx.core.djangoapps.embargo.test_utils import restrict_course -from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory @ddt.ddt @@ -75,17 +74,6 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase, OpenEdxEventsTestMixin) # Set up proctored exam self._create_proctored_exam(self.proctored_course) - course_run = CourseRunFactory.create(key=self.course.id) - course_run.update({ - 'min_effort': 1, - 'enrollment_count': 12345 - }) - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run - self.addCleanup(patch_course_data.stop) - def _create_proctored_exam(self, course): """ Helper function to create a proctored exam for a given course diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index a502f0f510..14a3c360e8 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -47,7 +47,6 @@ from openedx.core.djangoapps.content.course_overviews.tests.factories import Cou from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from xmodule.modulestore.tests.django_utils import ModuleStoreEnum, ModuleStoreTestCase, SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order from xmodule.data import CertificatesDisplayBehaviors # lint-amnesty, pylint: disable=wrong-import-order @@ -285,11 +284,6 @@ class DashboardTest(ModuleStoreTestCase, TestVerificationBase): self.client = Client() cache.clear() - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - self.addCleanup(patch_context.stop) - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def _check_verification_status_on(self, mode, value): """ @@ -657,7 +651,7 @@ class EnrollmentEventTestMixin(EventTestMixin): ) self.mock_segment_tracker.reset_mock() - def assert_enrollment_event_was_emitted(self, user, course_key, course, enrollment, course_run=None): + def assert_enrollment_event_was_emitted(self, user, course_key, course, enrollment): """Ensures an enrollment event was emitted since the last event related assertion""" self.mock_tracker.emit.assert_called_once_with( 'edx.course.enrollment.activated', @@ -668,8 +662,7 @@ class EnrollmentEventTestMixin(EventTestMixin): } ) self.mock_tracker.reset_mock() - properties, traits = self._build_segment_properties_and_traits(user, course_key, course, - enrollment, True, course_run) + properties, traits = self._build_segment_properties_and_traits(user, course_key, course, enrollment, True) self.mock_segment_tracker.track.assert_called_once_with( user.id, 'edx.course.enrollment.activated', properties, traits=traits ) @@ -692,43 +685,7 @@ class EnrollmentEventTestMixin(EventTestMixin): ) self.mock_segment_tracker.reset_mock() - @staticmethod - def get_course_date_blocks(): - """ - Create date list to test course dates - """ - today = datetime.now().replace(tzinfo=pytz.UTC) - yesterday = today - timedelta(days=1) - tomorrow = today + timedelta(days=1) - - course_date = { - 'title': 'Test Course', - 'assignment_type': '', - 'link': '', - 'assignment_count': -1, - 'due_time': '' - } - course_date_list = [ - { - 'date': yesterday.date(), - 'due_date': yesterday.strftime("%a, %b %d, %Y"), - **course_date - }, - { - 'date': today.date(), - 'due_date': today.strftime("%a, %b %d, %Y"), - **course_date - }, - { - 'date': tomorrow.date(), - 'due_date': tomorrow.strftime("%a, %b %d, %Y"), - **course_date - } - ] - return course_date_list - - def _build_segment_properties_and_traits(self, user, course_key, course, enrollment, - activated=False, course_run=None): + def _build_segment_properties_and_traits(self, user, course_key, course, enrollment, activated=False): """ Builds the segment properties and traits that are sent during enrollment events """ properties = { 'category': 'conversion', @@ -741,9 +698,6 @@ class EnrollmentEventTestMixin(EventTestMixin): traits = properties.copy() traits.update({'course_title': course.display_name, 'email': user.email}) - lms_root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL) - learning_base_url = configuration_helpers.get_value('LEARNING_MICROFRONTEND_URL', - settings.LEARNING_MICROFRONTEND_URL) if activated: properties.update({ 'email': user.email, @@ -753,72 +707,19 @@ class EnrollmentEventTestMixin(EventTestMixin): 'course_start': course.start, 'course_pacing': course.pacing, 'redesign_email': False, - 'price': 'Free', - 'goals_enabled': False, - 'learner_name': user.profile.name, - 'course_run_key': str(course_key), - 'lms_base_url': lms_root_url, - 'learning_base_url': learning_base_url, - 'course_title': course_run.get('title'), - 'short_description': course_run.get('short_description'), - 'marketing_url': course_run.get('marketing_url'), - 'pacing_type': course_run.get('pacing_type'), - 'partner_image_url': '', - 'banner_image_url': course_run.get('image').get('src'), - 'instructors': [], - 'instructors_count': 'even', - 'min_effort': course_run.get('min_effort'), - 'max_effort': course_run.get('max_effort'), - 'weeks_to_complete': course_run.get('weeks_to_complete'), - 'learners_count': '{:,}'.format(course_run.get('enrollment_count')), - 'course_date_blocks': self.get_course_date_blocks(), }) return properties, traits -@override_settings(PAID_COURSE_REGISTRATION_CURRENCY=["USD", "$"]) -class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, ModuleStoreTestCase): +class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase): """Tests enrolling and unenrolling in courses.""" - def setUp(self): - """ - Set up tests - """ - super().setUp() - - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = self.get_course_date_blocks() - self.addCleanup(patch_context.stop) - - @staticmethod - def _create_course_run(course_id, course): - """ - Discovery course run - """ - course_run = CourseRunFactory.create(key=course_id) - course_run.update({ - 'title': course.display_name, - 'short_description': course.short_description, - 'marketing_url': course.marketing_url, - 'pacing_type': 'self_paced' if course.self_paced else 'instructor_paced', - 'banner_image_url': course.banner_image_url, - 'min_effort': 1, - 'enrollment_count': 12345 - }) - return course_run - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_enrollment(self): user = UserFactory.create(username="joe", email="joe@joe.com", password="password") course_id = CourseKey.from_string("edX/Test101/2013") course_id_partial = CourseKey.from_string("edX/Test101/") course = CourseOverviewFactory.create(id=course_id) - course_run = self._create_course_run(course_id, course) - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run # Test basic enrollment assert not CourseEnrollment.is_enrolled(user, course_id) @@ -826,7 +727,7 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul enrollment = CourseEnrollment.enroll(user, course_id) assert CourseEnrollment.is_enrolled(user, course_id) assert CourseEnrollment.is_enrolled_by_partial(user, course_id_partial) - self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment, course_run) + self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment) # Enrolling them again should be harmless enrollment = CourseEnrollment.enroll(user, course_id) @@ -860,19 +761,12 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul enrollment = CourseEnrollment.enroll(user, course_id, "audit") assert CourseEnrollment.is_enrolled(user, course_id) assert enrollment.mode == 'audit' - self.addCleanup(patch_course_data.stop) - @override_settings(LEARNING_MICROFRONTEND_URL='https://learningmfe.openedx.org') def test_enrollment_non_existent_user(self): # Testing enrollment of newly unsaved user (i.e. no database entry) user = UserFactory(username="rusty", email="rusty@fake.edx.org") course_id = CourseLocator("edX", "Test101", "2013") course = CourseOverviewFactory.create(id=course_id) - course_run = self._create_course_run(course_id, course) - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run assert not CourseEnrollment.is_enrolled(user, course_id) @@ -884,23 +778,17 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul # should still work enrollment = CourseEnrollment.enroll(user, course_id) assert CourseEnrollment.is_enrolled(user, course_id) - self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment, course_run) - self.addCleanup(patch_course_data.stop) + self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_enrollment_by_email(self): user = UserFactory.create(username="jack", email="jack@fake.edx.org") course_id = CourseLocator("edX", "Test101", "2013") course = CourseOverviewFactory.create(id=course_id) - course_run = self._create_course_run(course_id, course) - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run enrollment = CourseEnrollment.enroll_by_email("jack@fake.edx.org", course_id) assert CourseEnrollment.is_enrolled(user, course_id) - self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment, course_run) + self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment) # This won't throw an exception, even though the user is not found assert CourseEnrollment.enroll_by_email('not_jack@fake.edx.org', course_id) is None @@ -928,7 +816,6 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul # Unenroll on non-existent user shouldn't throw an error CourseEnrollment.unenroll_by_email("not_jack@fake.edx.org", course_id) self.assert_no_events_were_emitted() - self.addCleanup(patch_course_data.stop) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_enrollment_multiple_classes(self): @@ -937,24 +824,11 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul course_id2 = CourseLocator("MITx", "6.003z", "2012") course1 = CourseOverviewFactory.create(id=course_id1) course2 = CourseOverviewFactory.create(id=course_id2) - course_run1 = self._create_course_run(course_id1, course1) - course_run2 = self._create_course_run(course_id2, course2) - - patch_course_data1 = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data1 = patch_course_data1.start() - course_data1.return_value = course_run1 enrollment1 = CourseEnrollment.enroll(user, course_id1) - self.assert_enrollment_event_was_emitted(user, course_id1, course1, enrollment1, course_run1) - self.addCleanup(course_data1.stop) - - patch_course_data2 = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data2 = patch_course_data2.start() - course_data2.return_value = course_run2 - + self.assert_enrollment_event_was_emitted(user, course_id1, course1, enrollment1) enrollment2 = CourseEnrollment.enroll(user, course_id2) - self.assert_enrollment_event_was_emitted(user, course_id2, course2, enrollment2, course_run2) - self.addCleanup(course_data2.stop) + self.assert_enrollment_event_was_emitted(user, course_id2, course2, enrollment2) assert CourseEnrollment.is_enrolled(user, course_id1) assert CourseEnrollment.is_enrolled(user, course_id2) @@ -973,11 +847,6 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul user = UserFactory.create(username="jack", email="jack@fake.edx.org") course_id = CourseLocator("edX", "Test101", "2013") course = CourseOverviewFactory.create(id=course_id) - course_run = self._create_course_run(course_id, course) - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run assert not CourseEnrollment.is_enrolled(user, course_id) # Creating an enrollment doesn't actually enroll a student @@ -989,7 +858,7 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul # Until you explicitly activate it enrollment.activate() assert CourseEnrollment.is_enrolled(user, course_id) - self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment, course_run) + self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment) # Activating something that's already active does nothing enrollment.activate() @@ -1010,22 +879,15 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul # for that user/course_id combination CourseEnrollment.enroll(user, course_id) assert CourseEnrollment.is_enrolled(user, course_id) - self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment, course_run) - self.addCleanup(course_data.stop) + self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment) - @override_settings(LEARNING_MICROFRONTEND_URL='https://learningmfe.openedx.org') def test_change_enrollment_modes(self): user = UserFactory.create(username="justin", email="jh@fake.edx.org") course_id = CourseLocator("edX", "Test101", "2013") course = CourseOverviewFactory.create(id=course_id) - course_run = self._create_course_run(course_id, course) - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run enrollment = CourseEnrollment.enroll(user, course_id, "audit") - self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment, course_run) + self.assert_enrollment_event_was_emitted(user, course_id, course, enrollment) enrollment = CourseEnrollment.enroll(user, course_id, "honor") self.assert_enrollment_mode_change_event_was_emitted(user, course_id, "honor", course, enrollment) @@ -1036,7 +898,6 @@ class EnrollInCourseTest(EnrollmentEventTestMixin, CacheIsolationTestCase, Modul enrollment = CourseEnrollment.enroll(user, course_id, "audit") self.assert_enrollment_mode_change_event_was_emitted(user, course_id, "audit", course, enrollment) - self.addCleanup(course_data.stop) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 4215dff245..a4af227fe8 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -76,11 +76,6 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT self.course = None self.ccx = None - patch_context = mock.patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - self.addCleanup(patch_context.stop) - def setup_course(self, size, enable_ccx, view_as_ccx): """ Build a gradable course where each node has `size` children. diff --git a/lms/djangoapps/course_home_api/outline/tests/test_view.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py index 8a45821908..c37788e9ee 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -46,10 +46,6 @@ class OutlineTabTestViews(BaseCourseHomeTests): def setUp(self): super().setUp() self.url = reverse('course-home:outline-tab', args=[self.course.id]) - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - self.addCleanup(patch_context.stop) def update_course_and_overview(self): self.update_course(self.course, self.user.id) diff --git a/lms/djangoapps/course_home_api/progress/tests/test_views.py b/lms/djangoapps/course_home_api/progress/tests/test_views.py index 6c7c43f03c..4c330c7cc3 100644 --- a/lms/djangoapps/course_home_api/progress/tests/test_views.py +++ b/lms/djangoapps/course_home_api/progress/tests/test_views.py @@ -47,11 +47,6 @@ class ProgressTabTestViews(BaseCourseHomeTests): super().setUp() self.url = reverse('course-home:progress-tab', args=[self.course.id]) - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - self.addCleanup(patch_context.stop) - def add_subsection_with_problem(self, **kwargs): """Makes a chapter -> problem chain, and sets up the subsection as requested, returning the problem""" chapter = ItemFactory(parent=self.course, category='chapter') diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index e549e62d3e..17c87108e2 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -338,10 +338,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): """ NUM_PROBLEMS = 20 - @patch('common.djangoapps.student.helpers.get_course_dates_for_email') - def test_index_query_counts(self, mock_course_dates_for_email): + def test_index_query_counts(self): # TODO: decrease query count as part of REVO-28 - mock_course_dates_for_email.return_value = [] ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) with self.store.default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() @@ -352,20 +350,6 @@ class IndexQueryTestCase(ModuleStoreTestCase): for _ in range(self.NUM_PROBLEMS): ItemFactory.create(category='problem', parent_location=vertical.location) - course_run = CourseRunFactory.create(key=course.id) - course_run['title'] = course.display_name - course_run['short_description'] = None - course_run['marketing_url'] = 'www.edx.org' - course_run['pacing_type'] = 'self_paced' - course_run['banner_image_url'] = '' - course_run['min_effort'] = 1 - course_run['enrollment_count'] = 12345 - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run - self.addCleanup(patch_course_data.stop) - self.client.login(username=self.user.username, password=self.user_password) CourseEnrollment.enroll(self.user, course.id) diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index b2102e9ed9..01eabf1531 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -31,16 +31,6 @@ class TestCourseGradeFactory(GradeTestBase): """ Test that CourseGrades are calculated properly """ - def setUp(self): - """ - setup tests - """ - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - super().setUp() - self.addCleanup(patch_context.stop) - def _assert_zero_grade(self, course_grade, expected_grade_class): """ Asserts whether the given course_grade is as expected with diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index ecc2be8fe5..6fbb4834db 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -374,9 +374,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): (ModuleStoreEnum.Type.split, 2, 48), ) @ddt.unpack - @patch('common.djangoapps.student.helpers.get_course_dates_for_email') - def test_query_counts(self, store_type, mongo_count, expected_query_count, mock_course_dates_for_email): - mock_course_dates_for_email.return_value = [] + def test_query_counts(self, store_type, mongo_count, expected_query_count): with self.store.default_store(store_type): experiment_group_a = Group(2, 'Expériment Group A') experiment_group_b = Group(3, 'Expériment Group B') diff --git a/openedx/core/djangoapps/credit/tests/test_services.py b/openedx/core/djangoapps/credit/tests/test_services.py index 315376a64b..7f6b384635 100644 --- a/openedx/core/djangoapps/credit/tests/test_services.py +++ b/openedx/core/djangoapps/credit/tests/test_services.py @@ -7,8 +7,6 @@ from unittest.mock import patch from datetime import datetime import ddt -from edx_django_utils.cache import RequestCache - from common.djangoapps.course_modes.models import CourseMode from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.credit.api.eligibility import set_credit_requirements @@ -63,10 +61,10 @@ class CreditServiceTests(ModuleStoreTestCase): Makes sure that get_credit_state returns None if the user's enrollment is inactive """ + enrollment = self.enroll() enrollment.is_active = False enrollment.save() - RequestCache.clear_all_namespaces() assert self.service.get_credit_state(self.user.id, self.course.id) is None diff --git a/openedx/core/djangoapps/enrollments/tests/test_views.py b/openedx/core/djangoapps/enrollments/tests/test_views.py index 0f38a38887..fca14e9c84 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_views.py +++ b/openedx/core/djangoapps/enrollments/tests/test_views.py @@ -42,7 +42,6 @@ from openedx.core.djangoapps.user_api.models import RetirementState, UserOrgTag, from openedx.core.lib.django_test_client_utils import get_absolute_url from openedx.features.enterprise_support.tests import FAKE_ENTERPRISE_CUSTOMER from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseServiceMockMixin -from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import CourseStaffRole from common.djangoapps.student.tests.factories import AdminFactory, SuperuserFactory, UserFactory @@ -104,12 +103,6 @@ class EnrollmentTestMixin: if as_server: extra['HTTP_X_EDX_API_KEY'] = self.API_KEY - course_run = self._create_course_run(course_id) - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course_run - self.addCleanup(patch_course_data.stop) - # Verify that the modulestore is queried as expected. with check_mongo_calls_range(min_finds=min_mongo_calls, max_finds=max_mongo_calls): with patch('openedx.core.djangoapps.enrollments.views.audit_log') as mock_audit_log: @@ -157,22 +150,6 @@ class EnrollmentTestMixin: resp = self.client.get(reverse("courseenrollments")) return json.loads(resp.content.decode('utf-8')) - @staticmethod - def _create_course_run(course_id): - """ - Discovery course run - """ - course_run = CourseRunFactory.create(key=course_id) - course_run.update({ - 'title': None, - 'short_description': None, - 'marketing_url': 'http://www.morales.com/', - 'pacing_type': 'self_paced', - 'min_effort': 1, - 'enrollment_count': 12345 - }) - return course_run - @override_settings(EDX_API_KEY="i am a key") @ddt.ddt @@ -217,10 +194,6 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente password=self.PASSWORD, ) self.client.login(username=self.USERNAME, password=self.PASSWORD) - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - self.addCleanup(patch_context.stop) @ddt.data( # Default (no course modes in the database) @@ -1194,11 +1167,6 @@ class EnrollmentEmbargoTest(EnrollmentTestMixin, UrlResetMixin, ModuleStoreTestC self.client.login(username=self.USERNAME, password=self.PASSWORD) self.url = reverse('courseenrollments') - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - self.addCleanup(patch_context.stop) - def _generate_data(self): return json.dumps({ 'course_details': { @@ -1410,14 +1378,9 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): password=self.PASSWORD, ) self.client.login(username=self.USERNAME, password=self.PASSWORD) - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] for course in self.courses: self.assert_enrollment_status(course_id=str(course.id), username=self.USERNAME, is_active=True) - self.addCleanup(patch_context.stop) - def _create_test_retirement(self, user=None): """ Helper method to create a RetirementStatus with useful defaults @@ -1554,10 +1517,6 @@ class UserRoleTest(ModuleStoreTestCase): is_staff=True, ) self.client.login(username=self.USERNAME, password=self.PASSWORD) - patch_context = patch('common.djangoapps.student.helpers.get_course_dates_for_email') - get_course = patch_context.start() - get_course.return_value = [] - self.addCleanup(patch_context.stop) def _create_expected_role_dict(self, course, role): """ Creates the expected role dict object that the view should return """ diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index 91fe2f40ed..20f9d5d7a9 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -4,7 +4,7 @@ Tests for schedules resolvers import datetime -from unittest.mock import patch, Mock +from unittest.mock import Mock import crum import ddt @@ -35,7 +35,6 @@ from openedx.core.djangoapps.schedules.resolvers import ( from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangolib.testing.utils import CacheIsolationMixin, skip_unless_lms -from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory class SchedulesResolverTestMixin(CacheIsolationMixin): @@ -66,22 +65,6 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): bin_num=2, ) - @staticmethod - def _course_run(course_id, course_overview): - """ - Discovery course run - """ - course_run = CourseRunFactory.create(key=course_id) - course_run.update({ - 'course_title': course_overview.display_name, - 'short_description': course_overview.short_description, - 'marketing_url': 'http://www.morales.com/', - 'pacing_type': 'self_paced', - 'min_effort': 1, - 'enrollment_count': 12345 - }) - return course_run - @ddt.data( 'course1' ) @@ -121,26 +104,12 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): assert result == mock_query.exclude.return_value @ddt.data(0, 1) - @patch('common.djangoapps.student.helpers.get_course_dates_for_email') - def test_external_course_updates(self, bucket, mock_course_dates_for_email): + def test_external_course_updates(self, bucket): """Confirm that we exclude enrollments in the external course updates experiment""" - mock_course_dates_for_email.return_value = [] - user = UserFactory() overview1 = CourseOverviewFactory(has_highlights=False) # set has_highlights just to avoid a modulestore lookup overview2 = CourseOverviewFactory(has_highlights=False) - course1 = self._course_run(overview1.id, overview1) - course2 = self._course_run(overview2.id, overview2) - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course1 - - patch_course_data = patch('openedx.core.djangoapps.catalog.api.get_course_run_details') - course_data = patch_course_data.start() - course_data.return_value = course2 - # We need to enroll with a request, because our specific experiment code expects it self.addCleanup(crum.set_current_request, None) request = RequestFactory().get(self.site) @@ -164,7 +133,6 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): else: assert len(schedules) == 2 assert {s.enrollment for s in schedules} == {enrollment1, enrollment2} - self.addCleanup(patch_course_data.stop) @skip_unless_lms diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index 7c22274e7c..ba16fdd6bd 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -643,13 +643,10 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase, MasqueradeMixin): # pyli (True, False), ) @ddt.unpack - @patch('common.djangoapps.student.helpers.get_course_dates_for_email') - def test_content_gating_holdback(self, put_user_in_holdback, is_gated, mock_course_dates_for_email): + def test_content_gating_holdback(self, put_user_in_holdback, is_gated): """ Test that putting a user in the content gating holdback disables content gating. """ - mock_course_dates_for_email.return_value = [] - user = UserFactory.create() enrollment = CourseEnrollment.enroll(user, self.course.id) if put_user_in_holdback: