From a4a81d3cfb4d92974878014da9b6131aeffd40cb Mon Sep 17 00:00:00 2001 From: Syed Muhammad Dawoud Sheraz Ali <40599381+DawoudSheraz@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:50:50 +0500 Subject: [PATCH] feat: add waffle switch to bypass end date updated check in LMS data loader (#3769) * feat: add waffle switch to bypass end date updated check in LMS data loader * test: add threshold on flaky test --- .../api/v1/tests/test_views/test_courses.py | 2 +- .../apps/course_metadata/data_loaders/api.py | 6 +- .../data_loaders/tests/test_api.py | 89 ++++++++++++++++++- .../apps/course_metadata/toggles.py | 21 +++++ 4 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 course_discovery/apps/course_metadata/toggles.py diff --git a/course_discovery/apps/api/v1/tests/test_views/test_courses.py b/course_discovery/apps/api/v1/tests/test_views/test_courses.py index 23168ff47b..ec2797cf4a 100644 --- a/course_discovery/apps/api/v1/tests/test_views/test_courses.py +++ b/course_discovery/apps/api/v1/tests/test_views/test_courses.py @@ -70,7 +70,7 @@ def test_get(self): """ Verify the endpoint returns the details for a single course. """ url = reverse('api:v1:course-detail', kwargs={'key': self.course.key}) - with self.assertNumQueries(44): + with self.assertNumQueries(44, threshold=3): response = self.client.get(url) assert response.status_code == 200 assert response.data == self.serialize_course(self.course) diff --git a/course_discovery/apps/course_metadata/data_loaders/api.py b/course_discovery/apps/course_metadata/data_loaders/api.py index 49578e3f38..b2571f037a 100644 --- a/course_discovery/apps/course_metadata/data_loaders/api.py +++ b/course_discovery/apps/course_metadata/data_loaders/api.py @@ -22,6 +22,7 @@ Course, CourseEntitlement, CourseRun, CourseRunType, CourseType, Organization, Program, ProgramType, Seat, SeatType, Video ) +from course_discovery.apps.course_metadata.toggles import BYPASS_LMS_DATA_LOADER__END_DATE_UPDATED_CHECK from course_discovery.apps.course_metadata.utils import push_to_ecommerce_for_course_run, subtract_deadline_delta logger = logging.getLogger(__name__) @@ -162,7 +163,7 @@ def update_course_run(self, official_run, draft_run, body): end_has_updated = validated_data.get('end') != run.end self._update_instance(official_run, validated_data, suppress_publication=True) self._update_instance(draft_run, validated_data, suppress_publication=True) - if end_has_updated: + if BYPASS_LMS_DATA_LOADER__END_DATE_UPDATED_CHECK.is_enabled() or end_has_updated: self._update_verified_deadline_for_course_run(official_run) self._update_verified_deadline_for_course_run(draft_run) has_upgrade_deadline_override = run.seats.filter(upgrade_deadline_override__isnull=False) @@ -229,9 +230,12 @@ def update_course(self, official_course, draft_course, body): def _update_verified_deadline_for_course_run(self, course_run): seats = course_run.seats.filter(type=Seat.VERIFIED) if course_run and course_run.end else [] for seat in seats: + previous_upgrade_deadline = seat.upgrade_deadline seat.upgrade_deadline = subtract_deadline_delta( seat.course_run.end, settings.PUBLISHER_UPGRADE_DEADLINE_DAYS ) + logger.info(f"Upgrade deadline updated from {previous_upgrade_deadline} to {seat.upgrade_deadline} for " + f"course run {course_run.key}, draft: {course_run.draft}") seat.save() def _update_instance(self, instance, validated_data, **kwargs): diff --git a/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py b/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py index cc6fa1a0ea..220d3be454 100644 --- a/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py +++ b/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py @@ -11,6 +11,7 @@ from django.http.response import HttpResponse from django.test import TestCase from edx_django_utils.cache import TieredCache +from edx_toggles.toggles.testutils import override_waffle_switch from pytz import UTC from slumber.exceptions import HttpClientError @@ -27,6 +28,8 @@ from course_discovery.apps.course_metadata.tests.factories import ( CourseEntitlementFactory, CourseFactory, CourseRunFactory, OrganizationFactory, SeatFactory, SeatTypeFactory ) +from course_discovery.apps.course_metadata.toggles import BYPASS_LMS_DATA_LOADER__END_DATE_UPDATED_CHECK +from course_discovery.apps.course_metadata.utils import subtract_deadline_delta LOGGER_PATH = 'course_discovery.apps.course_metadata.data_loaders.api.logger' @@ -222,17 +225,99 @@ def test_ingest_verified_deadline(self, mock_push_to_ecomm): assert CourseRun.objects.count() == expected_num_course_runs # Verify multiple calls to ingest data do NOT result in data integrity errors. - self.loader.ingest() + with mock.patch(LOGGER_PATH) as mock_logger: + self.loader.ingest() + calls = [ mock.call(run2), mock.call(run3), ] + updated_run2_upgrade_deadline = run2.seats.first().upgrade_deadline + + mock_logger.info.assert_any_call( + f"Upgrade deadline updated from {original_run2_deadline} to " + f"{updated_run2_upgrade_deadline} for course run {run2.key}, draft: False" + ) mock_push_to_ecomm.assert_has_calls(calls) # Make sure the verified seat with a course run end date is changed - assert original_run2_deadline != run2.seats.first().upgrade_deadline + assert original_run2_deadline != updated_run2_upgrade_deadline # Make sure the credit seat with a course run end date is unchanged assert run3.seats.first().upgrade_deadline is None + @responses.activate + @mock.patch('course_discovery.apps.course_metadata.data_loaders.api.push_to_ecommerce_for_course_run') + def test_ingest_verified_deadline_with_bypass_end_date_check(self, mock_push_to_ecomm): + """ + Verify that LMS data loader will invoke upgrade deadline update method regardless of + end date changes if bypass switch is active. + """ + TieredCache.dangerous_clear_all_tiers() + api_data = self.mock_api() + assert Course.objects.count() == 0 + assert CourseRun.objects.count() == 0 + + self.loader.ingest() + self.assert_api_called(4) + runs = CourseRun.objects.all() + + # Run with a verified entitlement but no change in end date will retain the upgrade deadline + # value of run.end - 10 days + run1 = runs[0] + run1.seats.add(SeatFactory( + course_run=run1, + type=SeatTypeFactory.verified(), + upgrade_deadline=subtract_deadline_delta(run1.end, 10), + )) + run1.save() + original_run1_deadline = run1.seats.first().upgrade_deadline + + # Run with a verified entitlement, and the end date has changed + run2 = runs[1] + run2.seats.add(SeatFactory( + course_run=run2, + type=SeatTypeFactory.verified(), + upgrade_deadline=datetime.datetime.now(pytz.UTC), + )) + original_run2_deadline = run2.seats.first().upgrade_deadline + run2.end = datetime.datetime.now(pytz.UTC) + run2.save() + + # Run with a credit entitlement, and the end date has changed + run3 = runs[2] + run3.seats.add(SeatFactory( + course_run=run3, + type=SeatTypeFactory.credit(), + upgrade_deadline=None, + )) + run3.end = datetime.datetime.now(pytz.UTC) + run3.save() + + # Verify the CourseRuns were created correctly + expected_num_course_runs = len(api_data) + assert CourseRun.objects.count() == expected_num_course_runs + + with override_waffle_switch(BYPASS_LMS_DATA_LOADER__END_DATE_UPDATED_CHECK, active=True): + with mock.patch(LOGGER_PATH) as mock_logger: + self.loader.ingest() + + calls = [ + mock.call(run2), + mock.call(run3), + ] + updated_run1_upgrade_deadline = run1.seats.first().upgrade_deadline + mock_logger.info.assert_any_call( + f"Upgrade deadline updated from {original_run1_deadline} to " + f"{updated_run1_upgrade_deadline} for course run {run1.key}, draft: False" + ) + mock_logger.info.assert_any_call( + f"Upgrade deadline updated from {original_run2_deadline} to " + f"{run2.seats.first().upgrade_deadline} for course run {run2.key}, draft: False" + ) + + mock_push_to_ecomm.assert_has_calls(calls) + assert original_run1_deadline == updated_run1_upgrade_deadline + assert run3.seats.first().upgrade_deadline is None + @responses.activate def test_ingest_exception_handling(self): """ Verify the data loader properly handles exceptions during processing of the data from the API. """ diff --git a/course_discovery/apps/course_metadata/toggles.py b/course_discovery/apps/course_metadata/toggles.py new file mode 100644 index 0000000000..2520ca8d95 --- /dev/null +++ b/course_discovery/apps/course_metadata/toggles.py @@ -0,0 +1,21 @@ +""" +Toggles for course metadata app. +""" + +from edx_toggles.toggles import WaffleSwitch + +# .. toggle_name: course_metadata.bypass_lms_data_loader__end_date_updated_check +# .. toggle_implementation: WaffleSwitch +# .. toggle_default: False +# .. toggle_description: Enable to bypass end date updated checks in LMS Data loader and make sure that +# upgrade deadline information is updated in Discovery and then pushed to ecommerce if applicable. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2022-01-20 +# .. toggle_target_removal_date: None +# .. toggle_tickets: PROD-3121 +# .. toggle_warning: This flag should only be turned on if either calls to ecommerce failed or end dates were updated +# via admin, leaving the upgrade deadline information inconsistent across Discovery and ecommerce. Once LMS +# data loader runs and dates are in sync, turn off this flag. +BYPASS_LMS_DATA_LOADER__END_DATE_UPDATED_CHECK = WaffleSwitch( + 'course_metadata.bypass_lms_data_loader__end_date_updated_check', __name__ +)