From 47e49b4989c777618aa11ccc2fd444e2e24ca894 Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Mon, 27 Jan 2025 16:57:11 +0500 Subject: [PATCH 1/3] feat: marke course as active is is_marketable_external --- .../apps/api/v1/tests/test_utils.py | 50 +++++++++++++++++++ enterprise_catalog/apps/api/v1/utils.py | 4 +- .../apps/catalog/content_metadata_utils.py | 4 +- .../apps/catalog/tests/test_algolia_utils.py | 26 ++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/enterprise_catalog/apps/api/v1/tests/test_utils.py b/enterprise_catalog/apps/api/v1/tests/test_utils.py index 51185d1b2..d7179d275 100644 --- a/enterprise_catalog/apps/api/v1/tests/test_utils.py +++ b/enterprise_catalog/apps/api/v1/tests/test_utils.py @@ -5,6 +5,7 @@ from enterprise_catalog.apps.api.v1.utils import ( get_archived_content_count, get_most_recent_modified_time, + is_course_run_active, ) from enterprise_catalog.apps.catalog.tests.factories import ( ContentMetadataFactory, @@ -82,3 +83,52 @@ def test_get_archived_content_count(self): [highlighted_content_1, highlighted_content_2, highlighted_content_3] ) assert archived_content_count == 2 + + def test_is_course_run_active(self): + """ + Test that the is_course_run_active function correctly identifies active course runs. + """ + # Test case where course run is active + active_course_run = { + 'is_enrollable': True, + 'is_marketable_external': True, + 'status': 'published', + 'is_marketable': True + } + assert is_course_run_active(active_course_run) is True + + # Test case where course run is not active due to not being enrollable + not_enrollable_course_run = { + 'is_enrollable': False, + 'is_marketable_external': True, + 'status': 'published', + 'is_marketable': True + } + assert is_course_run_active(not_enrollable_course_run) is False + + # Test case where course is not is_marketable but is_marketable_external + not_marketable_course_run = { + 'is_enrollable': True, + 'is_marketable_external': True, + 'status': 'published', + 'is_marketable': False + } + assert is_course_run_active(not_marketable_course_run) is True + + # Test case where course run is not active due to not being published + not_published_course_run = { + 'is_enrollable': False, + 'is_marketable_external': False, + 'status': 'archived', + 'is_marketable': True + } + assert is_course_run_active(not_published_course_run) is False + + # Test case where course run is active due to being marketable externally + marketable_external_course_run = { + 'is_enrollable': True, + 'is_marketable_external': True, + 'status': 'reviewed', + 'is_marketable': False + } + assert is_course_run_active(marketable_external_course_run) is True diff --git a/enterprise_catalog/apps/api/v1/utils.py b/enterprise_catalog/apps/api/v1/utils.py index aead4a5f6..71afe313e 100644 --- a/enterprise_catalog/apps/api/v1/utils.py +++ b/enterprise_catalog/apps/api/v1/utils.py @@ -70,9 +70,11 @@ def is_course_run_active(course_run): Returns: bool: True if course run is "active" """ + is_enrollable = course_run.get('is_enrollable', False) + if course_run.get("is_marketable_external") and is_enrollable: + return True course_run_status = course_run.get('status') or '' is_published = course_run_status.lower() == 'published' - is_enrollable = course_run.get('is_enrollable', False) is_marketable = course_run.get('is_marketable', False) return is_published and is_enrollable and is_marketable diff --git a/enterprise_catalog/apps/catalog/content_metadata_utils.py b/enterprise_catalog/apps/catalog/content_metadata_utils.py index 60739e279..5cead3f2f 100644 --- a/enterprise_catalog/apps/catalog/content_metadata_utils.py +++ b/enterprise_catalog/apps/catalog/content_metadata_utils.py @@ -71,9 +71,11 @@ def is_course_run_active(course_run): Returns: bool: True if course run is "active" """ + is_enrollable = course_run.get('is_enrollable', False) + if course_run.get("is_marketable_external") and is_enrollable: + return True course_run_status = course_run.get('status') or '' is_published = course_run_status.lower() == 'published' - is_enrollable = course_run.get('is_enrollable', False) is_marketable = course_run.get('is_marketable', False) return is_published and is_enrollable and is_marketable diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 469c07fba..87b079159 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -119,6 +119,18 @@ class AlgoliaUtilsTests(TestCase): 'enrollment_end': days_from_now(30), 'restriction_type': RESTRICTION_FOR_B2B, }, + # A Exec-Ed course, not is_marketable but is_marketable_external. + { + 'expected_result': True, + 'course_type': EXEC_ED_2U_COURSE_TYPE, + 'start': days_from_now(1), + 'end': days_from_now(30), + 'enrollment_end': days_from_now(1), + 'is_marketable': False, + 'is_marketable_external': True, + 'advertised_course_run_status': 'reviewed', + 'course_run_availability': 'Upcoming' + }, ) @ddt.unpack def test_should_index_course( @@ -130,6 +142,7 @@ def test_should_index_course( advertised_course_run_status='published', is_enrollable=True, is_marketable=True, + is_marketable_external=True, course_run_availability='current', seats=None, course_type=COURSE, @@ -155,6 +168,7 @@ def test_should_index_course( 'status': advertised_course_run_status, 'is_enrollable': is_enrollable, 'is_marketable': is_marketable, + 'is_marketable_external': is_marketable_external, 'availability': course_run_availability, 'seats': seats or [], 'start': start, @@ -783,6 +797,18 @@ def test_get_course_runs(self, searchable_course, expected_course_runs): }, ['Archived'], ), + ( + { + 'course_runs': [{ + 'status': 'reviewed', + 'is_enrollable': True, + 'is_marketable': False, + 'is_marketable_external': True, + 'availability': 'Upcoming' + }] + }, + ['Upcoming'], + ), ) @ddt.unpack def test_get_course_availability(self, course_metadata, expected_availability): From ea7d8c7c4658a60c0a3108adf95ff8b17f8b8896 Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Tue, 28 Jan 2025 13:38:44 +0500 Subject: [PATCH 2/3] refactor: and remove duplicate function --- enterprise_catalog/apps/api/v1/utils.py | 25 +++---------------- .../apps/catalog/content_metadata_utils.py | 12 ++++++--- 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/enterprise_catalog/apps/api/v1/utils.py b/enterprise_catalog/apps/api/v1/utils.py index 71afe313e..f1b45eeed 100644 --- a/enterprise_catalog/apps/api/v1/utils.py +++ b/enterprise_catalog/apps/api/v1/utils.py @@ -10,6 +10,10 @@ urlunsplit, ) +from enterprise_catalog.apps.catalog.content_metadata_utils import ( + is_course_run_active, +) + logger = logging.getLogger(__name__) @@ -59,27 +63,6 @@ def get_enterprise_utm_context(enterprise_name): return utm_context -def is_course_run_active(course_run): - """ - Checks whether a course run is active. That is, whether the course run is published, - enrollable, and marketable. - - Arguments: - course_run (dict): The metadata about a course run. - - Returns: - bool: True if course run is "active" - """ - is_enrollable = course_run.get('is_enrollable', False) - if course_run.get("is_marketable_external") and is_enrollable: - return True - course_run_status = course_run.get('status') or '' - is_published = course_run_status.lower() == 'published' - is_marketable = course_run.get('is_marketable', False) - - return is_published and is_enrollable and is_marketable - - def is_any_course_run_active(course_runs): """ Iterates over all course runs to check if there any course run that is available for enrollment. diff --git a/enterprise_catalog/apps/catalog/content_metadata_utils.py b/enterprise_catalog/apps/catalog/content_metadata_utils.py index 5cead3f2f..046dc8016 100644 --- a/enterprise_catalog/apps/catalog/content_metadata_utils.py +++ b/enterprise_catalog/apps/catalog/content_metadata_utils.py @@ -66,19 +66,23 @@ def is_course_run_active(course_run): """ Checks whether a course run is active. That is, whether the course run is published, enrollable, and marketable. + Checking is_published in case of is_marketable_external is redundant because + the Discovery service already handles the status behind is_marketable_external + property. Arguments: course_run (dict): The metadata about a course run. Returns: bool: True if course run is "active" """ - is_enrollable = course_run.get('is_enrollable', False) - if course_run.get("is_marketable_external") and is_enrollable: - return True course_run_status = course_run.get('status') or '' is_published = course_run_status.lower() == 'published' is_marketable = course_run.get('is_marketable', False) + is_enrollable = course_run.get('is_enrollable', False) + + is_marketable_internal = is_published and is_marketable + is_marketable_external = course_run.get("is_marketable_external", False) - return is_published and is_enrollable and is_marketable + return is_enrollable and (is_marketable_internal or is_marketable_external) def get_course_first_paid_enrollable_seat_price(course): From 1beacd08d6c2f3616e35bad4f292d84db0fed595 Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Tue, 28 Jan 2025 17:42:17 +0500 Subject: [PATCH 3/3] chore: add debug log --- enterprise_catalog/apps/catalog/algolia_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 77278708d..caf2d8f33 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -1253,6 +1253,7 @@ def _get_is_active_course_run(full_course_run): f'[_get_is_active_course_run] course run is not active ' f'key: {full_course_run.get("key")}, ' f'is_marketable: {full_course_run.get("is_marketable")}, ' + f'is_marketable_external: {full_course_run.get("is_marketable_external")}, ' f'is_enrollable: {full_course_run.get("is_enrollable")}, ' f'availability: {availability}, ' f'status: {full_course_run.get("status")}'