diff --git a/course_discovery/apps/course_metadata/management/commands/populate_executive_education_data_csv.py b/course_discovery/apps/course_metadata/management/commands/populate_executive_education_data_csv.py index 0b95bc9bf9..8820fdc877 100644 --- a/course_discovery/apps/course_metadata/management/commands/populate_executive_education_data_csv.py +++ b/course_discovery/apps/course_metadata/management/commands/populate_executive_education_data_csv.py @@ -152,8 +152,10 @@ def handle(self, *args, **options): # pylint: disable=too-many-statements if getsmarter_flag: product['organization'] = map_external_org_code_to_internal_org_code( product['universityAbbreviation'], product_source) - if product.get('variants'): + if 'variants' in product: variants = product.pop('variants') + if not variants: + logger.warning(f"Skipping product {product['name']} ingestion as it has no variants") for variant in variants: product.update({'variant': variant}) output_dict = self.get_transformed_data(row, product) @@ -292,7 +294,6 @@ def get_transformed_data(self, partially_filled_csv_dict, product_dict): 'start_time': '00:00:00', 'end_time': '00:00:00', 'reg_close_time': '00:00:00', - 'publish_date': date.today().isoformat(), 'course_level': 'Introductory', 'course_pacing': 'Instructor-Paced', 'content_language': language, @@ -313,6 +314,10 @@ def get_transformed_data(self, partially_filled_csv_dict, product_dict): 'organization_short_code_override': product_dict[ 'altUniversityAbbreviation' ], + 'publish_date': ( + partially_filled_csv_dict.get('start_date') or product_dict['variant']['startDate'] + if product_dict['variant'].get('status') == 'scheduled' else date.today().isoformat() + ), '2u_organization_code': product_dict['universityAbbreviation'], 'number': product_dict['abbreviation'], 'alternate_number': product_dict['altAbbreviation'], diff --git a/course_discovery/apps/course_metadata/management/commands/tests/test_populate_executive_education_data_csv.py b/course_discovery/apps/course_metadata/management/commands/tests/test_populate_executive_education_data_csv.py index f6727fb5d7..117c880f68 100644 --- a/course_discovery/apps/course_metadata/management/commands/tests/test_populate_executive_education_data_csv.py +++ b/course_discovery/apps/course_metadata/management/commands/tests/test_populate_executive_education_data_csv.py @@ -7,6 +7,7 @@ from datetime import date from tempfile import NamedTemporaryFile +import ddt import mock import responses from django.conf import settings @@ -20,6 +21,7 @@ LOGGER_PATH = 'course_discovery.apps.course_metadata.management.commands.populate_executive_education_data_csv' +@ddt.ddt class TestPopulateExecutiveEducationDataCsv(CSVLoaderMixin, TestCase): """ Test suite for populate_executive_education_data_csv management command. @@ -191,6 +193,41 @@ def test_successful_file_data_population_with_getsmarter_flag(self, mock_get_sma ), ) + @mock.patch("course_discovery.apps.course_metadata.utils.GetSmarterEnterpriseApiClient") + def test_skip_products_ingestion_if_variants_data_empty(self, mock_get_smarter_client): + """ + Verify that the command skips the product ingestion if the variants data is empty + """ + success_api_response = copy.deepcopy(self.SUCCESS_API_RESPONSE_V2) + success_api_response["products"][0]["variants"] = [] + mock_get_smarter_client.return_value.request.return_value.json.return_value = ( + self.mock_get_smarter_client_response( + override_get_smarter_client_response=success_api_response + ) + ) + with NamedTemporaryFile() as output_csv: + with LogCapture(LOGGER_PATH) as log_capture: + call_command( + "populate_executive_education_data_csv", + "--output_csv", + output_csv.name, + "--use_getsmarter_api_client", + True, + ) + log_capture.check_present( + ( + LOGGER_PATH, + "WARNING", + f"Skipping product {success_api_response['products'][0]['name']} " + f"ingestion as it has no variants", + ), + ) + + output_csv.seek(0) + with open(output_csv.name, "r") as csv_file: + reader = csv.DictReader(csv_file) + assert not any(reader) + @mock.patch('course_discovery.apps.course_metadata.utils.GetSmarterEnterpriseApiClient') def test_successful_file_data_population_with_getsmarter_flag_with_multiple_variants(self, mock_get_smarter_client): """ @@ -247,6 +284,49 @@ def test_successful_file_data_population_with_getsmarter_flag_with_multiple_vari ), ) + @mock.patch("course_discovery.apps.course_metadata.utils.GetSmarterEnterpriseApiClient") + @ddt.data( + ("active", str(date.today().isoformat())), ("scheduled", "2024-03-20") + ) + @ddt.unpack + def test_successful_file_data_population_with_getsmarter_flag_with_future_variants( + self, + variant_status, + expected_publish_date, + mock_get_smarter_client, + ): + """ + Verify that data is correctly populated from the API response when the getsmarter flag is enabled. + If a variant is scheduled, its publish date is set to the start date. If the variant is active, + the publish date is set to the current date. + """ + success_api_response = copy.deepcopy(self.SUCCESS_API_RESPONSE_V2) + success_api_response["products"][0]["variants"][0]["status"] = variant_status + success_api_response["products"][0]["variants"][1]["status"] = variant_status + + mock_get_smarter_client.return_value.request.return_value.json.return_value = ( + self.mock_get_smarter_client_response( + override_get_smarter_client_response=success_api_response + ) + ) + + with NamedTemporaryFile() as output_csv: + call_command( + 'populate_executive_education_data_csv', + '--output_csv', output_csv.name, + '--use_getsmarter_api_client', True, + ) + + output_csv.seek(0) + with open(output_csv.name, 'r') as csv_file: + reader = csv.DictReader(csv_file) + + data_row = next(reader) + assert data_row['Publish Date'] == expected_publish_date + + data_row = next(reader) + assert data_row['Publish Date'] == expected_publish_date + @responses.activate def test_successful_file_data_population_with_input_csv(self): """ diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index c0dad00d84..655c8a0974 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -2611,13 +2611,6 @@ def availability(self): now = datetime.datetime.now(pytz.UTC) upcoming_cutoff = now + datetime.timedelta(days=60) - # Use external course availability for ExecEd course run types - # Check additional_metadata existence as a fail safe for some unit tests - # where this flow is triggered via signals before AdditionalMetadata object can be persisted - if self.type.slug in [CourseRunType.UNPAID_EXECUTIVE_EDUCATION, CourseRunType.PAID_EXECUTIVE_EDUCATION] and \ - self.course.additional_metadata: - return self.external_course_availability - if self.has_ended(now): return _('Archived') elif self.start and (self.start <= now): diff --git a/course_discovery/apps/course_metadata/tests/test_models.py b/course_discovery/apps/course_metadata/tests/test_models.py index 4dcfd1b6ee..14208a2542 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/course_discovery/apps/course_metadata/tests/test_models.py @@ -1109,7 +1109,10 @@ def test_availability(self, start, end, expected_availability): ) @ddt.unpack def test_external_course_availability(self, product_status, expected_availability): - """ Verify the property returns the appropriate availability string based on external product status """ + """ + Verify that the course availability is determined based on the start and end dates, + not on the product status, for a given course run. + """ course_run = factories.CourseRunFactory( type=factories.CourseRunTypeFactory( slug=CourseRunType.UNPAID_EXECUTIVE_EDUCATION @@ -1118,7 +1121,13 @@ def test_external_course_availability(self, product_status, expected_availabilit additional_metadata=AdditionalMetadataFactory( product_status=product_status ) - ) + ), + start=(datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=5)), + end=( + datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=100) + if product_status.value != "archived" + else datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=10) + ), ) assert course_run.availability == expected_availability