From 3dc19328c3d656974a9254f9f56612884139bac4 Mon Sep 17 00:00:00 2001 From: Ali Nawaz Date: Tue, 25 Feb 2025 00:32:27 +0500 Subject: [PATCH 1/6] chore: swap translation_languages with ai_languages --- course_discovery/apps/api/serializers.py | 4 ++-- .../apps/api/tests/test_serializers.py | 2 +- .../apps/course_metadata/admin.py | 2 +- .../apps/course_metadata/algolia_models.py | 23 +++++++++++++------ .../apps/course_metadata/index.py | 10 ++++---- .../apps/course_metadata/tests/factories.py | 5 +++- .../tests/test_algolia_models.py | 21 ++++++++++++----- .../apps/course_metadata/tests/test_models.py | 17 ++++++++------ 8 files changed, 54 insertions(+), 30 deletions(-) diff --git a/course_discovery/apps/api/serializers.py b/course_discovery/apps/api/serializers.py index bbef4995c4..caeed00a81 100644 --- a/course_discovery/apps/api/serializers.py +++ b/course_discovery/apps/api/serializers.py @@ -1103,10 +1103,10 @@ class Meta(MinimalCourseRunSerializer.Meta): 'first_enrollable_paid_seat_price', 'has_ofac_restrictions', 'ofac_comment', 'enrollment_count', 'recent_enrollment_count', 'expected_program_type', 'expected_program_name', 'course_uuid', 'estimated_hours', 'content_language_search_facet_name', 'enterprise_subscription_inclusion', - 'transcript_languages_search_facet_names', 'translation_languages' + 'transcript_languages_search_facet_names', 'ai_languages' ) read_only_fields = ('enrollment_count', 'recent_enrollment_count', 'content_language_search_facet_name', - 'enterprise_subscription_inclusion', 'translation_languages') + 'enterprise_subscription_inclusion', 'ai_languages') def get_instructors(self, obj): # pylint: disable=unused-argument # This field is deprecated. Use the staff field. diff --git a/course_discovery/apps/api/tests/test_serializers.py b/course_discovery/apps/api/tests/test_serializers.py index f013b3dfa5..277eed8fb6 100644 --- a/course_discovery/apps/api/tests/test_serializers.py +++ b/course_discovery/apps/api/tests/test_serializers.py @@ -782,7 +782,7 @@ def get_expected_data(cls, course_run, request): 'transcript_languages_search_facet_names': [ lang.get_search_facet_display() for lang in course_run.transcript_languages.all() ], - 'translation_languages': course_run.translation_languages, + 'ai_languages': course_run.ai_languages, }) return expected diff --git a/course_discovery/apps/course_metadata/admin.py b/course_discovery/apps/course_metadata/admin.py index ef1827cdd7..cd3f30a0bb 100644 --- a/course_discovery/apps/course_metadata/admin.py +++ b/course_discovery/apps/course_metadata/admin.py @@ -313,7 +313,7 @@ class CourseRunAdmin(SimpleHistoryAdmin): raw_id_fields = ('course', 'draft_version',) readonly_fields = [ 'enrollment_count', 'recent_enrollment_count', 'hidden', 'key', 'enterprise_subscription_inclusion', - 'variant_id', 'fixed_price_usd', 'translation_languages' + 'variant_id', 'fixed_price_usd', 'ai_languages' ] search_fields = ('uuid', 'key', 'title_override', 'course__title', 'slug', 'external_key', 'variant_id') save_error = False diff --git a/course_discovery/apps/course_metadata/algolia_models.py b/course_discovery/apps/course_metadata/algolia_models.py index 200b7659a9..951e4183e8 100644 --- a/course_discovery/apps/course_metadata/algolia_models.py +++ b/course_discovery/apps/course_metadata/algolia_models.py @@ -83,7 +83,7 @@ def delegate_attributes(cls): 'secondary_description', 'tertiary_description'] facet_fields = ['availability_level', 'subject_names', 'levels', 'active_languages', 'staff_slugs', 'product_allowed_in', 'product_blocked_in', 'learning_type', 'learning_type_exp', - 'product_translation_languages'] + 'product_ai_languages'] ranking_fields = ['availability_rank', 'product_recent_enrollment_count', 'promoted_in_spanish_index', 'product_value_per_click_usa', 'product_value_per_click_international', 'product_value_per_lead_usa', 'product_value_per_lead_international'] @@ -355,10 +355,16 @@ def product_max_effort(self): return getattr(self.advertised_course_run, 'max_effort', None) @property - def product_translation_languages(self): - if self.advertised_course_run and self.advertised_course_run.translation_languages: - return self.advertised_course_run.translation_languages - return [] + def product_ai_languages(self): + if ai_langs:=(self.advertised_course_run and self.advertised_course_run.ai_languages): + return { + 'translation_languages': [lang['label'] for lang in ai_langs['translation_languages']], + 'transcription_languages': [lang['label'] for lang in ai_langs['transcription_languages']] + } + return { + 'translation_languages': [], + 'transcription_languages': [] + } @property def owners(self): @@ -544,8 +550,11 @@ def product_max_effort(self): return self.max_hours_effort_per_week @property - def product_translation_languages(self): - return [] + def product_ai_languages(self): + return { + 'translation_languages': [], + 'transcription_languages': [] + } @property def subject_names(self): diff --git a/course_discovery/apps/course_metadata/index.py b/course_discovery/apps/course_metadata/index.py index 7617601665..ce7dfc609c 100644 --- a/course_discovery/apps/course_metadata/index.py +++ b/course_discovery/apps/course_metadata/index.py @@ -81,7 +81,7 @@ class EnglishProductIndex(BaseProductIndex): ('staff_slugs', 'staff'), ('product_allowed_in', 'allowed_in'), ('product_blocked_in', 'blocked_in'), 'subscription_eligible', 'subscription_prices', 'learning_type', 'learning_type_exp', - ('product_translation_languages', 'translation_languages')) + ('product_ai_languages', 'ai_languages')) ranking_fields = ('availability_rank', ('product_recent_enrollment_count', 'recent_enrollment_count'), ('product_value_per_click_usa', 'value_per_click_usa'), ('product_value_per_click_international', 'value_per_click_international'), @@ -117,7 +117,7 @@ class EnglishProductIndex(BaseProductIndex): 'partner', 'availability', 'subject', 'level', 'language', 'product', 'program_type', 'filterOnly(staff)', 'filterOnly(allowed_in)', 'filterOnly(blocked_in)', 'skills.skill', 'skills.category', 'skills.subcategory', 'tags', 'subscription_eligible', 'subscription_prices', - 'learning_type', 'learning_type_exp', 'translation_languages.code', 'translation_languages.label', + 'learning_type', 'learning_type_exp', 'ai_languages.translation_languages', 'ai_languages.transcription_languages', ], 'customRanking': ['asc(availability_rank)', 'desc(recent_enrollment_count)'] } @@ -135,7 +135,7 @@ class SpanishProductIndex(BaseProductIndex): ('staff_slugs', 'staff'), ('product_allowed_in', 'allowed_in'), ('product_blocked_in', 'blocked_in'), 'subscription_eligible', 'subscription_prices', 'learning_type', 'learning_type_exp', - ('product_translation_languages', 'translation_languages')) + ('product_ai_languages', 'ai_languages')) ranking_fields = ('availability_rank', ('product_recent_enrollment_count', 'recent_enrollment_count'), ('product_value_per_click_usa', 'value_per_click_usa'), ('product_value_per_click_international', 'value_per_click_international'), @@ -173,8 +173,8 @@ class SpanishProductIndex(BaseProductIndex): 'partner', 'availability', 'subject', 'level', 'language', 'product', 'program_type', 'filterOnly(staff)', 'filterOnly(allowed_in)', 'filterOnly(blocked_in)', 'skills.skill', 'skills.category', 'skills.subcategory', 'tags', 'subscription_eligible', - 'subscription_prices', 'learning_type', 'learning_type_exp', 'translation_languages.code', - 'translation_languages.label', + 'subscription_prices', 'learning_type', 'learning_type_exp', 'ai_languages.translation_languages', + 'ai_languages.transcription_languages', ], 'customRanking': ['desc(promoted_in_spanish_index)', 'asc(availability_rank)', 'desc(recent_enrollment_count)'] } diff --git a/course_discovery/apps/course_metadata/tests/factories.py b/course_discovery/apps/course_metadata/tests/factories.py index 824e9e8928..f1c3ed02e4 100644 --- a/course_discovery/apps/course_metadata/tests/factories.py +++ b/course_discovery/apps/course_metadata/tests/factories.py @@ -482,7 +482,10 @@ class CourseRunFactory(SalesforceRecordFactory): type = factory.SubFactory(CourseRunTypeFactory) variant_id = factory.LazyFunction(uuid4) fixed_price_usd = FuzzyDecimal(0.0, 650.0) - translation_languages = [{'code': 'fr', 'label': 'French'}] + ai_languages = { + 'translation_languages': [{'code': 'fr', 'label': 'French'}], + 'transcription_languages': [{'code': 'en', 'label': 'English'}, {'code': 'bs', 'label': 'Bosnian'}] + } @factory.post_generation def staff(self, create, extracted, **kwargs): diff --git a/course_discovery/apps/course_metadata/tests/test_algolia_models.py b/course_discovery/apps/course_metadata/tests/test_algolia_models.py index 1789087ba9..c07a22d505 100644 --- a/course_discovery/apps/course_metadata/tests/test_algolia_models.py +++ b/course_discovery/apps/course_metadata/tests/test_algolia_models.py @@ -579,13 +579,19 @@ def test_learning_type_exp_non_open_course(self, course_type_slug, expected_resu course.type = CourseTypeFactory(slug=course_type_slug) assert course.learning_type_exp == [expected_result] - def test_course_translation_languages(self): + def test_course_ai_languages(self): course = self.create_current_upgradeable_course() - assert course.product_translation_languages == [{'code': 'fr', 'label': 'French'}] + assert course.product_ai_languages == { + 'translation_languages': ['French'], + 'transcription_languages': ['English', 'Bosnian'] + } - def test_course_translation_languages__no_advertised_run(self): + def test_course_ai_languages__no_advertised_run(self): course = self.create_blocked_course(status=CourseRunStatus.Unpublished) - assert course.product_translation_languages == [] + assert course.product_ai_languages == { + 'translation_languages': [], + 'transcription_languages': [] + } @ddt.ddt @@ -927,6 +933,9 @@ def test_learning_type_exp(self, program_type_slug, learning_type): program = AlgoliaProxyProgramFactory(partner=self.__class__.edxPartner, type=program_type) assert program.learning_type_exp == [learning_type] - def test_program_translation_languages(self): + def test_program_ai_languages(self): program = AlgoliaProxyProgramFactory(partner=self.__class__.edxPartner) - assert program.product_translation_languages == [] + assert program.product_ai_languages == { + 'translation_languages': [], + 'transcription_languages': [] + } diff --git a/course_discovery/apps/course_metadata/tests/test_models.py b/course_discovery/apps/course_metadata/tests/test_models.py index 53de73d2a6..73e74d8bb9 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/course_discovery/apps/course_metadata/tests/test_models.py @@ -990,16 +990,19 @@ def test_course_run_fixed_usd_price(self): ) assert course_run.fixed_price_usd is None - def test_course_run_translation_languages(self): + def test_course_run_ai_languages(self): """ - Sanity checks for the translation_languages field + Sanity checks for the ai_languages field """ - DEFAULT_TRANSLATION_LANGUAGES = [{'code': 'fr', 'label': 'French'}] course_run = factories.CourseRunFactory() - assert course_run.translation_languages == DEFAULT_TRANSLATION_LANGUAGES - - course_run = factories.CourseRunFactory(translation_languages=None) - assert course_run.translation_languages is None + DEFAULT_AI_LANGUAGES = { + 'translation_languages': [{'code': 'fr', 'label': 'French'}], + 'transcription_languages': [{'code': 'en', 'label': 'English'}, {'code': 'bs', 'label': 'Bosnian'}] + } + assert course_run.ai_languages == DEFAULT_AI_LANGUAGES + + course_run = factories.CourseRunFactory(ai_languages=None) + assert course_run.ai_languages is None @ddt.data('full_description_override', 'outcome_override', 'short_description_override') def test_html_fields_are_validated(self, field_name): From b191d425cacd1ada7a5e43514fdd4d8dd2b1ce57 Mon Sep 17 00:00:00 2001 From: Ali Nawaz Date: Tue, 25 Feb 2025 11:57:01 +0500 Subject: [PATCH 2/6] chore: mgmt command changes --- course_discovery/apps/core/api_client/lms.py | 12 +- .../apps/core/tests/test_api_clients.py | 34 ++-- .../tests/test_update_course_ai_languages.py | 167 ++++++++++++++++++ .../test_update_course_ai_translations.py | 167 ------------------ ...tions.py => update_course_ai_languages.py} | 37 ++-- course_discovery/settings/base.py | 2 +- 6 files changed, 219 insertions(+), 200 deletions(-) create mode 100644 course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py delete mode 100644 course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_translations.py rename course_discovery/apps/course_metadata/management/commands/{update_course_ai_translations.py => update_course_ai_languages.py} (56%) diff --git a/course_discovery/apps/core/api_client/lms.py b/course_discovery/apps/core/api_client/lms.py index 916e0cf6d3..4a7b99e1c7 100644 --- a/course_discovery/apps/core/api_client/lms.py +++ b/course_discovery/apps/core/api_client/lms.py @@ -210,17 +210,17 @@ def get_blocks_metadata(self, block_id: str, **kwargs): cache_key = get_cache_key(block_id=block_id, resource=resource) return self._get_blocks_data(block_id, cache_key, query_parameters, resource) - def get_course_run_translations(self, course_run_id: str): + def get_course_run_translations_and_transcriptions(self, course_run_id: str): """ - Get translation information for a given course run. + Get translation and transcription information for a given course run. Args: - course_run_id (str): The course run ID to fetch translation information for. + course_run_id (str): The course run ID to fetch information for. Returns: - dict: A dictionary containing the translation information or an empty dict on error. + dict: A dictionary containing the relevant information or an empty dict on error. """ - resource = settings.LMS_API_URLS['translations'] + resource = settings.LMS_API_URLS['translations_and_transcriptions'] resource_url = urljoin(self.lms_url, resource) try: @@ -228,5 +228,5 @@ def get_course_run_translations(self, course_run_id: str): response.raise_for_status() return response.json() except RequestException as e: - logger.exception(f'Failed to fetch translation data for course run [{course_run_id}]: {e}') + logger.exception(f'Failed to fetch data for course run [{course_run_id}]: {e}') return {} diff --git a/course_discovery/apps/core/tests/test_api_clients.py b/course_discovery/apps/core/tests/test_api_clients.py index 738b3d6868..671bffe587 100644 --- a/course_discovery/apps/core/tests/test_api_clients.py +++ b/course_discovery/apps/core/tests/test_api_clients.py @@ -237,35 +237,43 @@ def test_get_blocks_data_cache_hit(self): assert len(responses.calls) == 1 @responses.activate - def test_get_course_run_translations(self): + def test_get_course_run_translations_and_transcriptions(self): """ - Verify that `get_course_run_translations` returns correct translation data. + Verify that `get_course_run_translations_and_transcriptions` returns correct data. """ course_run_id = 'course-v1:edX+DemoX+Demo_Course' - translation_data = { - "en": {"title": "Course Title", "language": "English"}, - "fr": {"title": "Titre du cours", "language": "French"} + response_data = { + "available_translation_languages": [ + { + "code": "ar", + "enabled": True, + "label": "Arabic" + } + ], + "feature_enabled": True, + "feature_available": False, + "transcription_languages": ["en", "fr"] } - resource = settings.LMS_API_URLS['translations'] + resource = settings.LMS_API_URLS['translations_and_transcriptions'] resource_url = urljoin(self.partner.lms_url, resource) responses.add( responses.GET, resource_url, - json=translation_data, + json=response_data, status=200 ) - result = self.lms.get_course_run_translations(course_run_id) + result = self.lms.get_course_run_translations_and_transcriptions(course_run_id) assert result == translation_data @responses.activate - def test_get_course_run_translations_with_error(self): + def test_get_course_run_translations_and_transcriptions_with_error(self): """ - Verify that get_course_run_translations returns an empty dictionary when there's an error. + Verify that `get_course_run_translations_and_transcriptions` returns an empty dictionary when there's an error. """ course_run_id = 'course-v1:edX+DemoX+Demo_Course' - resource = settings.LMS_API_URLS['translations'] + resource = settings.LMS_API_URLS['translations_and_transcriptions'] resource_url = urljoin(self.partner.lms_url, resource) responses.add( @@ -274,6 +282,6 @@ def test_get_course_run_translations_with_error(self): status=500 ) - result = self.lms.get_course_run_translations(course_run_id) + result = self.lms.get_course_run_translations_and_transcriptions(course_run_id) assert result == {} - assert 'Failed to fetch translation data for course run [%s]' % course_run_id in self.log_messages['error'][0] + assert 'Failed to fetch data for course run [%s]' % course_run_id in self.log_messages['error'][0] diff --git a/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py b/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py new file mode 100644 index 0000000000..ea3955c159 --- /dev/null +++ b/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py @@ -0,0 +1,167 @@ +""" +Unit tests for the `update_course_ai_languages` management command. +""" +import datetime +from unittest.mock import patch + +import ddt +from django.core.management import CommandError, call_command +from django.test import TestCase +from django.utils.timezone import now + +from course_discovery.apps.course_metadata.models import CourseRun, CourseRunType +from course_discovery.apps.course_metadata.tests.factories import CourseRunFactory, PartnerFactory, SeatFactory + + +@ddt.ddt +@patch('course_discovery.apps.core.api_client.lms.LMSAPIClient.get_course_run_translations_and_transcriptions') +class UpdateCourseAiLanguagesTests(TestCase): + """Test Suite for the update_course_ai_languages management command.""" + + AI_LANGUAGES_DATA = { + 'available_translation_languages': [ + {'code': 'fr', 'label': 'French'}, + {'code': 'es', 'label': 'Spanish'} + ], + 'feature_enabled': True, + } + + AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS = { + **AI_LANGUAGES_DATA, + 'transcription_languages': ['en', 'fr'] + } + + def setUp(self): + self.partner = PartnerFactory() + self.course_run = CourseRunFactory() + + def assert_ai_langs(self, run, data): + self.assertListEqual( + run.ai_languages['translation_languages'], + data['available_translation_languages'] + ) + self.assertListEqual( + run.ai_languages['transcription_languages'], + data.get('transcription_languages', []) + ) + + + @ddt.data(AI_LANGUAGES_DATA, AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS) + def test_update_ai_languages(self, mock_data, mock_get_translations_and_transcriptions): + """Test the command with a valid course run and response data.""" + mock_get_translations_and_transcriptions.return_value = mock_data + + call_command('update_course_ai_languages', partner=self.partner.name) + + course_run = CourseRun.objects.get(id=self.course_run.id) + self.assert_ai_langs(course_run, mock_data) + + @ddt.data(AI_LANGUAGES_DATA, AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS) + def test_update_ai_languages_draft(self, mock_data, mock_get_translations_and_transcriptions): + """ + Test the command with both draft and non-draft course runs, ensuring that the both draft and non-draft + course runs are updated appropriately. + """ + mock_get_translations_and_transcriptions.return_value = mock_data + draft_course_run = CourseRunFactory( + draft=True, end=now() + datetime.timedelta(days=10) + ) + course_run = CourseRunFactory(draft=False, draft_version_id=draft_course_run.id) + + call_command('update_course_ai_languages', partner=self.partner.name) + + course_run.refresh_from_db() + self.assert_ai_langs(course_run, mock_data) + + draft_course_run.refresh_from_db() + self.assert_ai_langs(draft_course_run, mock_data) + + + @ddt.data(AI_LANGUAGES_DATA, AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS) + def test_update_ai_languages_no_translations(self, mock_data, mock_get_translations_and_transcriptions): + """Test the command when no translations are returned for a course run.""" + mock_get_translations_and_transcriptions.return_value = { + **mock_data, + 'available_translation_languages': [], + } + + call_command('update_course_ai_languages', partner=self.partner.name) + + course_run = CourseRun.objects.get(id=self.course_run.id) + self.assertListEqual(course_run.ai_languages['translation_languages'], []) + + @ddt.data(AI_LANGUAGES_DATA, AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS) + def test_command_with_active_flag(self, mock_data, mock_get_translations_and_transcriptions): + """Test the command with the active flag filtering active course runs.""" + mock_get_translations_and_transcriptions.return_value = mock_data + + active_course_run = CourseRunFactory(end=now() + datetime.timedelta(days=10), ai_languages=None) + non_active_course_run = CourseRunFactory(end=now() - datetime.timedelta(days=10), ai_languages=None) + + call_command('update_course_ai_languages', partner=self.partner.name, active=True) + + active_course_run.refresh_from_db() + non_active_course_run.refresh_from_db() + + self.assert_ai_langs(active_course_run, mock_data) + assert non_active_course_run.ai_languages is None + + @ddt.data(AI_LANGUAGES_DATA, AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS) + def test_command_with_marketable_flag(self, mock_data, mock_get_translations_and_transcriptions): + """Test the command with the marketable flag filtering marketable course runs.""" + mock_get_translations_and_transcriptions.return_value = mock_data + + verified_and_audit_type = CourseRunType.objects.get(slug='verified-audit') + verified_and_audit_type.is_marketable = True + verified_and_audit_type.save() + + marketable_course_run = CourseRunFactory( + status='published', + slug='test-course-run', + type=verified_and_audit_type, + ai_languages=None + ) + seat = SeatFactory(course_run=marketable_course_run) + marketable_course_run.seats.add(seat) + + call_command('update_course_ai_languages', partner=self.partner.name, marketable=True) + + marketable_course_run.refresh_from_db() + self.assert_ai_langs(marketable_course_run, mock_data) + + @ddt.data(AI_LANGUAGES_DATA, AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS) + def test_command_with_marketable_and_active_flag(self, mock_data, mock_get_translations_and_transcriptions): + """Test the command with the marketable and active flag filtering both marketable and active course runs.""" + mock_get_translations_and_transcriptions.return_value = mock_data + + non_active_non_marketable_course_run = CourseRunFactory( + end=now() - datetime.timedelta(days=10), ai_languages=None + ) + active_non_marketable_course_run = CourseRunFactory( + end=now() + datetime.timedelta(days=10), ai_languages=None + ) + + verified_and_audit_type = CourseRunType.objects.get(slug='verified-audit') + verified_and_audit_type.is_marketable = True + verified_and_audit_type.save() + + marketable_non_active_course_run = CourseRunFactory( + status='published', + slug='test-course-run', + type=verified_and_audit_type, + end=now() - datetime.timedelta(days=10), ai_languages=None + ) + seat = SeatFactory(course_run=marketable_non_active_course_run) + marketable_non_active_course_run.seats.add(seat) + + call_command('update_course_ai_languages', partner=self.partner.name, marketable=True, active=True) + + marketable_non_active_course_run.refresh_from_db() + self.assert_ai_langs(marketable_non_active_course_run, mock_data) + self.assert_ai_langs(active_non_marketable_course_run, mock_data) + assert non_active_non_marketable_course_run.ai_languages is None + + def test_command_no_partner(self, _): + """Test the command raises an error if no valid partner is found.""" + with self.assertRaises(CommandError): + call_command('update_course_ai_languages', partner='nonexistent-partner') diff --git a/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_translations.py b/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_translations.py deleted file mode 100644 index 55e50439ce..0000000000 --- a/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_translations.py +++ /dev/null @@ -1,167 +0,0 @@ -""" -Unit tests for the `update_course_ai_translations` management command. -""" -import datetime -from unittest.mock import patch - -from django.core.management import CommandError, call_command -from django.test import TestCase -from django.utils.timezone import now - -from course_discovery.apps.course_metadata.models import CourseRun, CourseRunType -from course_discovery.apps.course_metadata.tests.factories import CourseRunFactory, PartnerFactory, SeatFactory - - -@patch('course_discovery.apps.core.api_client.lms.LMSAPIClient.get_course_run_translations') -class UpdateCourseAiTranslationsTests(TestCase): - """Test Suite for the update_course_ai_translations management command.""" - - TRANSLATION_DATA = { - 'available_translation_languages': [ - {'code': 'fr', 'label': 'French'}, - {'code': 'es', 'label': 'Spanish'} - ], - 'feature_enabled': True - } - - def setUp(self): - self.partner = PartnerFactory() - self.course_run = CourseRunFactory() - - def test_update_course_run_translations(self, mock_get_translations): - """Test the command with a valid course run and translation data.""" - mock_get_translations.return_value = self.TRANSLATION_DATA - - call_command('update_course_ai_translations', partner=self.partner.name) - - course_run = CourseRun.objects.get(id=self.course_run.id) - self.assertListEqual( - course_run.translation_languages, - self.TRANSLATION_DATA['available_translation_languages'] - ) - - def test_update_course_run_translations_draft(self, mock_get_translations): - """ - Test the command with both draft and non-draft course runs, ensuring that the both draft and non-draft - course runs are updated with the available translation languages. - """ - mock_get_translations.return_value = self.TRANSLATION_DATA - draft_course_run = CourseRunFactory( - draft=True, end=now() + datetime.timedelta(days=10) - ) - course_run = CourseRunFactory(draft=False, draft_version_id=draft_course_run.id) - - call_command("update_course_ai_translations", partner=self.partner.name) - - course_run.refresh_from_db() - self.assertListEqual( - course_run.translation_languages, self.TRANSLATION_DATA["available_translation_languages"], - ) - - draft_course_run.refresh_from_db() - self.assertListEqual( - draft_course_run.translation_languages, self.TRANSLATION_DATA["available_translation_languages"], - ) - - def test_command_with_no_translations(self, mock_get_translations): - """Test the command when no translations are returned for a course run.""" - mock_get_translations.return_value = { - **self.TRANSLATION_DATA, - 'available_translation_languages': [], - 'feature_enabled': False - } - - call_command('update_course_ai_translations', partner=self.partner.name) - - course_run = CourseRun.objects.get(id=self.course_run.id) - self.assertListEqual(course_run.translation_languages, []) - - def test_command_with_active_flag(self, mock_get_translations): - """Test the command with the active flag filtering active course runs.""" - mock_get_translations.return_value = { - **self.TRANSLATION_DATA, - 'available_translation_languages': [{'code': 'fr', 'label': 'French'}] - } - - active_course_run = CourseRunFactory(end=now() + datetime.timedelta(days=10)) - non_active_course_run = CourseRunFactory(end=now() - datetime.timedelta(days=10), translation_languages=[]) - - call_command('update_course_ai_translations', partner=self.partner.name, active=True) - - active_course_run.refresh_from_db() - non_active_course_run.refresh_from_db() - - self.assertListEqual( - active_course_run.translation_languages, - [{'code': 'fr', 'label': 'French'}] - ) - self.assertListEqual(non_active_course_run.translation_languages, []) - - def test_command_with_marketable_flag(self, mock_get_translations): - """Test the command with the marketable flag filtering marketable course runs.""" - mock_get_translations.return_value = { - **self.TRANSLATION_DATA, - 'available_translation_languages': [{'code': 'es', 'label': 'Spanish'}] - } - - verified_and_audit_type = CourseRunType.objects.get(slug='verified-audit') - verified_and_audit_type.is_marketable = True - verified_and_audit_type.save() - - marketable_course_run = CourseRunFactory( - status='published', - slug='test-course-run', - type=verified_and_audit_type - ) - seat = SeatFactory(course_run=marketable_course_run) - marketable_course_run.seats.add(seat) - - call_command('update_course_ai_translations', partner=self.partner.name, marketable=True) - - marketable_course_run.refresh_from_db() - self.assertListEqual( - marketable_course_run.translation_languages, - [{'code': 'es', 'label': 'Spanish'}] - ) - - def test_command_with_marketable_and_active_flag(self, mock_get_translations): - """Test the command with the marketable and active flag filtering both marketable and active course runs.""" - mock_get_translations.return_value = { - **self.TRANSLATION_DATA, - 'available_translation_languages': [{'code': 'fr', 'label': 'French'}] - } - - non_active_non_marketable_course_run = CourseRunFactory( - end=now() - datetime.timedelta(days=10), translation_languages=[]) - active_non_marketable_course_run = CourseRunFactory(end=now() + datetime.timedelta(days=10)) - - verified_and_audit_type = CourseRunType.objects.get(slug='verified-audit') - verified_and_audit_type.is_marketable = True - verified_and_audit_type.save() - - marketable_non_active_course_run = CourseRunFactory( - status='published', - slug='test-course-run', - type=verified_and_audit_type, - end=now() - datetime.timedelta(days=10), translation_languages=[] - ) - seat = SeatFactory(course_run=marketable_non_active_course_run) - marketable_non_active_course_run.seats.add(seat) - - call_command('update_course_ai_translations', partner=self.partner.name, marketable=True, active=True) - - marketable_non_active_course_run.refresh_from_db() - self.assertListEqual( - marketable_non_active_course_run.translation_languages, - [{'code': 'fr', 'label': 'French'}] - ) - self.assertListEqual( - active_non_marketable_course_run.translation_languages, - [{'code': 'fr', 'label': 'French'}] - ) - self.assertListEqual(non_active_non_marketable_course_run.translation_languages, []) - - def test_command_no_partner(self, _): - """Test the command raises an error if no valid partner is found.""" - with self.assertRaises(CommandError): - call_command('update_course_ai_translations', partner='nonexistent-partner') diff --git a/course_discovery/apps/course_metadata/management/commands/update_course_ai_translations.py b/course_discovery/apps/course_metadata/management/commands/update_course_ai_languages.py similarity index 56% rename from course_discovery/apps/course_metadata/management/commands/update_course_ai_translations.py rename to course_discovery/apps/course_metadata/management/commands/update_course_ai_languages.py index 8cbd5826b0..75a8d6bd3f 100644 --- a/course_discovery/apps/course_metadata/management/commands/update_course_ai_translations.py +++ b/course_discovery/apps/course_metadata/management/commands/update_course_ai_languages.py @@ -1,5 +1,5 @@ """ -Management command to fetch translation information from the LMS and update the CourseRun model. +Management command to fetch translation and transcription information from the LMS and update the CourseRun model. """ import logging @@ -14,7 +14,7 @@ class Command(BaseCommand): - help = 'Fetches Content AI Translations metadata from the LMS and updates the CourseRun model in Discovery.' + help = 'Fetches Content AI Translations and Transcriptions metadata from the LMS and updates the CourseRun model in Discovery.' def add_arguments(self, parser): parser.add_argument( @@ -39,7 +39,7 @@ def add_arguments(self, parser): def handle(self, *args, **options): """ - Example usage: ./manage.py update_course_ai_translations --partner=edx --active --marketable + Example usage: ./manage.py update_course_ai_languages --partner=edx --active --marketable """ partner_identifier = options.get('partner') partner = Partner.objects.filter(name__iexact=partner_identifier).first() @@ -60,20 +60,31 @@ def handle(self, *args, **options): for course_run in course_runs: try: - translation_data = lms_api_client.get_course_run_translations(course_run.key) - - course_run.translation_languages = ( - translation_data.get('available_translation_languages', []) - if translation_data.get('feature_enabled', False) + ai_languages_data = lms_api_client.get_course_run_translations_and_transcriptions(course_run.key) + available_translation_languages = ( + ai_languages_data.get('available_translation_languages', []) + if ai_languages_data.get('feature_enabled', False) else [] ) - course_run.save(update_fields=["translation_languages"]) + available_transcription_languages = ai_languages_data.get('transcription_languages', []) + + # Remove any keys other than `code` and `label` + available_translation_languages = [{'code': lang['code'], 'label': lang['label']} for lang in available_translation_languages] + + # Add the labels for the codes. Currently we set the code as the label. We will be fixing this in a follow-up PR + available_transcription_languages = [{'code': lang, 'label': lang} for lang in available_transcription_languages] + + course_run.ai_languages = { + "translation_languages": available_translation_languages, + "transcription_languages": available_transcription_languages + } + course_run.save(update_fields=["ai_languages"]) if course_run.draft_version: - course_run.draft_version.translation_languages = course_run.translation_languages - course_run.draft_version.save(update_fields=["translation_languages"]) - logger.info(f'Updated translations for {course_run.key} (both draft and non-draft versions)') + course_run.draft_version.ai_languages = course_run.ai_languages + course_run.draft_version.save(update_fields=["ai_languages"]) + logger.info(f'Updated ai languages for {course_run.key} (both draft and non-draft versions)') else: - logger.info(f'Updated translations for {course_run.key} (non-draft version only)') + logger.info(f'Updated ai languages for {course_run.key} (non-draft version only)') except Exception as e: # pylint: disable=broad-except logger.error(f'Error processing {course_run.key}: {e}') diff --git a/course_discovery/settings/base.py b/course_discovery/settings/base.py index 143f76205a..63e3e918ee 100644 --- a/course_discovery/settings/base.py +++ b/course_discovery/settings/base.py @@ -753,7 +753,7 @@ 'api_access_request': 'api-admin/api/v1/api_access_request/', 'blocks': 'api/courses/v1/blocks/', 'block_metadata': 'api/courses/v1/block_metadata/', - 'translations': 'api/translatable_xblocks/config/', + 'translations_and_transcriptions': 'api/translatable_xblocks/config/', } # Map defining the required data fields against courses types and course's product source. From 641a2a3d96a7e0823a2b0a80e9c434385ae86958 Mon Sep 17 00:00:00 2001 From: Ali Nawaz Date: Tue, 25 Feb 2025 12:38:17 +0500 Subject: [PATCH 3/6] test: fixes --- course_discovery/apps/core/tests/test_api_clients.py | 2 +- .../commands/tests/test_update_course_ai_languages.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/course_discovery/apps/core/tests/test_api_clients.py b/course_discovery/apps/core/tests/test_api_clients.py index 671bffe587..dfdf7a5426 100644 --- a/course_discovery/apps/core/tests/test_api_clients.py +++ b/course_discovery/apps/core/tests/test_api_clients.py @@ -265,7 +265,7 @@ def test_get_course_run_translations_and_transcriptions(self): ) result = self.lms.get_course_run_translations_and_transcriptions(course_run_id) - assert result == translation_data + assert result == response_data @responses.activate def test_get_course_run_translations_and_transcriptions_with_error(self): diff --git a/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py b/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py index ea3955c159..bead6ba8e6 100644 --- a/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py +++ b/course_discovery/apps/course_metadata/management/commands/tests/test_update_course_ai_languages.py @@ -21,14 +21,14 @@ class UpdateCourseAiLanguagesTests(TestCase): AI_LANGUAGES_DATA = { 'available_translation_languages': [ {'code': 'fr', 'label': 'French'}, - {'code': 'es', 'label': 'Spanish'} + {'code': 'cs', 'label': 'Czech'} ], 'feature_enabled': True, } AI_LANGUAGES_DATA_WITH_TRANSCRIPTIONS = { **AI_LANGUAGES_DATA, - 'transcription_languages': ['en', 'fr'] + 'transcription_languages': ['da', 'fr'] } def setUp(self): @@ -42,7 +42,7 @@ def assert_ai_langs(self, run, data): ) self.assertListEqual( run.ai_languages['transcription_languages'], - data.get('transcription_languages', []) + [{'code': lang_code, 'label': lang_code} for lang_code in data.get('transcription_languages', [])] ) @@ -157,6 +157,8 @@ def test_command_with_marketable_and_active_flag(self, mock_data, mock_get_trans call_command('update_course_ai_languages', partner=self.partner.name, marketable=True, active=True) marketable_non_active_course_run.refresh_from_db() + active_non_marketable_course_run.refresh_from_db() + non_active_non_marketable_course_run.refresh_from_db() self.assert_ai_langs(marketable_non_active_course_run, mock_data) self.assert_ai_langs(active_non_marketable_course_run, mock_data) assert non_active_non_marketable_course_run.ai_languages is None From 4a95b4acb8ec47a4f0b519b490110b3fc816a66e Mon Sep 17 00:00:00 2001 From: Ali Nawaz Date: Tue, 25 Feb 2025 15:11:58 +0500 Subject: [PATCH 4/6] temp: experiment with schema validation --- .../apps/course_metadata/models.py | 5 ++- .../apps/course_metadata/tests/test_models.py | 15 +++++++ .../apps/course_metadata/utils.py | 40 +++++++++++++++++++ requirements/base.in | 1 + 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index fecc480df0..17d6bb8f91 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -61,7 +61,7 @@ from course_discovery.apps.course_metadata.utils import ( UploadToFieldNamePath, clean_query, clear_slug_request_cache_for_course, custom_render_variations, get_course_run_statuses, get_slug_for_course, is_ocm_course, push_to_ecommerce_for_course_run, - push_tracks_to_lms_for_course_run, set_official_state, subtract_deadline_delta + push_tracks_to_lms_for_course_run, set_official_state, subtract_deadline_delta, validate_ai_languages ) from course_discovery.apps.ietf_language_tags.models import LanguageTag from course_discovery.apps.ietf_language_tags.utils import serialize_language @@ -2922,6 +2922,7 @@ def save(self, *args, suppress_publication=False, send_emails=True, **kwargs): suppress_publication (bool): if True, we won't push the run data to the marketing site send_emails (bool): whether to send email notifications for status changes from this save """ + self.clean() push_to_marketing = (not suppress_publication and self.course.partner.has_marketing_site and waffle.switch_is_active('publish_course_runs_to_marketing_site') and @@ -3531,6 +3532,8 @@ def clean(self): self.enrollment_count = 0 if self.recent_enrollment_count is None: self.recent_enrollment_count = 0 + if self.ai_languages is not None: + validate_ai_languages(self.ai_languages) @property def is_program_eligible_for_one_click_purchase(self): diff --git a/course_discovery/apps/course_metadata/tests/test_models.py b/course_discovery/apps/course_metadata/tests/test_models.py index 73e74d8bb9..a40730730f 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/course_discovery/apps/course_metadata/tests/test_models.py @@ -1004,6 +1004,21 @@ def test_course_run_ai_languages(self): course_run = factories.CourseRunFactory(ai_languages=None) assert course_run.ai_languages is None + def test_course_run_ai_languages_invalid(self): + """ + Check that trying to add a value for ai_languages that does not match the schema + raises + """ + course_run = factories.CourseRunFactory() + INVALID_AI_LANGUAGES = { + 'translation_languages': [{'code': 'fr', 'label': 'French', 'enabled': True}], + 'transcription_languages': [{'code': 'en', 'label': 'English'}] + } + course_run.ai_languages = INVALID_AI_LANGUAGES + + with pytest.raises(ValidationError): + course_run.save() + @ddt.data('full_description_override', 'outcome_override', 'short_description_override') def test_html_fields_are_validated(self, field_name): # Happy path diff --git a/course_discovery/apps/course_metadata/utils.py b/course_discovery/apps/course_metadata/utils.py index 603ee1698d..eff8a5ba19 100644 --- a/course_discovery/apps/course_metadata/utils.py +++ b/course_discovery/apps/course_metadata/utils.py @@ -26,6 +26,7 @@ from stdimage.models import StdImageFieldFile from taxonomy.utils import get_whitelisted_serialized_skills +from jsonschema.validators import Draft202012Validator from course_discovery.apps.core.models import SalesforceConfiguration from course_discovery.apps.core.utils import serialize_datetime from course_discovery.apps.course_metadata.choices import CourseRunStatus @@ -1262,3 +1263,42 @@ def get_course_run_statuses(statuses, course_runs): else: statuses.add(course_run.status) return statuses + +AI_LANG_SCHEMA = { + "type": "object", + "properties": { + "translation_languages": { + "type": "array", + "items": { + "type": "object", + "properties": { + "code": {"type": "string"}, + "label": {"type": "string"} + }, + "required": ["code", "label"], + "additionalProperties": False + } + }, + "transcription_languages": { + "type": "array", + "items": { + "type": "object", + "properties": { + "code": {"type": "string"}, + "label": {"type": "string"} + }, + "required": ["code", "label"], + "additionalProperties": False + + } + }, + }, + "required": ["translation_languages", "transcription_languages"], + "additionalProperties": False +} +Draft202012Validator.check_schema(payload_schema) +def validate_ai_languages(ai_langs): + try: + Draft202012Validator(AI_LANG_SCHEMA).validate(ai_langs) + except Exception as exc: + raise ValidationError("Could not validate ai_languages field") diff --git a/requirements/base.in b/requirements/base.in index 371653a531..57e164fac7 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -66,6 +66,7 @@ gspread html2text lxml[html_clean] jsonfield +jsonschema markdown openedx-atlas pillow From 0434db34c9ddddb0d004b48134a8577141836461 Mon Sep 17 00:00:00 2001 From: Ali Nawaz Date: Tue, 25 Feb 2025 15:30:26 +0500 Subject: [PATCH 5/6] fix: typo --- course_discovery/apps/course_metadata/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/course_discovery/apps/course_metadata/utils.py b/course_discovery/apps/course_metadata/utils.py index eff8a5ba19..5e0769df53 100644 --- a/course_discovery/apps/course_metadata/utils.py +++ b/course_discovery/apps/course_metadata/utils.py @@ -1296,7 +1296,7 @@ def get_course_run_statuses(statuses, course_runs): "required": ["translation_languages", "transcription_languages"], "additionalProperties": False } -Draft202012Validator.check_schema(payload_schema) +Draft202012Validator.check_schema(AI_LANG_SCHEMA) def validate_ai_languages(ai_langs): try: Draft202012Validator(AI_LANG_SCHEMA).validate(ai_langs) From b11dc4b13a1722e46e1678d0783d9ddd01577b47 Mon Sep 17 00:00:00 2001 From: Ali Nawaz Date: Tue, 25 Feb 2025 16:10:49 +0500 Subject: [PATCH 6/6] fix: typos --- course_discovery/apps/course_metadata/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index 17d6bb8f91..b3d37f9fcc 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -2450,6 +2450,8 @@ def clean(self): self.enrollment_count = 0 if self.recent_enrollment_count is None: self.recent_enrollment_count = 0 + if self.ai_languages is not None: + validate_ai_languages(self.ai_languages) @property def first_enrollable_paid_seat_price(self): @@ -3532,8 +3534,6 @@ def clean(self): self.enrollment_count = 0 if self.recent_enrollment_count is None: self.recent_enrollment_count = 0 - if self.ai_languages is not None: - validate_ai_languages(self.ai_languages) @property def is_program_eligible_for_one_click_purchase(self):