diff --git a/course_discovery/apps/api/tests/test_utils.py b/course_discovery/apps/api/tests/test_utils.py index 0cd51f8022..1cda0d83a8 100644 --- a/course_discovery/apps/api/tests/test_utils.py +++ b/course_discovery/apps/api/tests/test_utils.py @@ -169,7 +169,7 @@ def test_generate_data_for_studio_api_without_team(self): mock_logger.assert_called_with( 'No course team admin specified for course [%s]. This may result in a Studio course run ' 'being created without a course team.', - run.key.split('/')[1] + run.key.split('+')[1] ) def test_calculate_course_run_key_run_value_with_multiple_runs_per_trimester(self): diff --git a/course_discovery/apps/api/v1/tests/test_views/test_catalog_queries.py b/course_discovery/apps/api/v1/tests/test_views/test_catalog_queries.py index cfb025f9fd..f7c2c95408 100644 --- a/course_discovery/apps/api/v1/tests/test_views/test_catalog_queries.py +++ b/course_discovery/apps/api/v1/tests/test_views/test_catalog_queries.py @@ -13,7 +13,7 @@ def setUp(self): self.user = UserFactory(is_staff=True, is_superuser=True) self.client.force_authenticate(self.user) self.course = CourseFactory(partner=self.partner, key='simple_key') - self.course_run = CourseRunFactory(course=self.course) + self.course_run = CourseRunFactory(course=self.course, key='simple/key/run') self.url_base = reverse('api:v1:catalog-query_contains') self.error_message = 'CatalogQueryContains endpoint requires query and identifiers list(s)' diff --git a/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py b/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py index c68f500032..41e09b5a90 100644 --- a/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py +++ b/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py @@ -1,6 +1,7 @@ import datetime import urllib from unittest import mock +from urllib.parse import urlencode import ddt import pytest @@ -1031,7 +1032,7 @@ def test_filter_by_keys(self): CourseRun.objects.all().delete() expected = CourseRunFactory.create_batch(3, course__partner=self.partner) keys = ','.join([course.key for course in expected]) - url = '{root}?keys={keys}'.format(root=reverse('api:v1:course_run-list'), keys=keys) + url = '{root}?{params}'.format(root=reverse('api:v1:course_run-list'), params=urlencode({'keys': keys})) self.assert_list_results(url, expected) def test_filter_by_marketable(self): 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 97ad39b106..4af715cfbe 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 @@ -1,5 +1,6 @@ import datetime from unittest import mock +from urllib.parse import urlencode import ddt import pytest @@ -269,7 +270,7 @@ def test_list_key_filter(self): """ Verify the endpoint returns a list of courses filtered by the specified keys. """ courses = CourseFactory.create_batch(3, partner=self.partner) keys = ','.join([course.key for course in courses]) - url = '{root}?keys={keys}'.format(root=reverse('api:v1:course-list'), keys=keys) + url = '{root}?{params}'.format(root=reverse('api:v1:course-list'), params=urlencode({'keys': keys})) with self.assertNumQueries(49): response = self.client.get(url) diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index fe2403a421..f9bafd092b 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -21,6 +21,7 @@ from django_extensions.db.models import TimeStampedModel from elasticsearch.exceptions import RequestError from elasticsearch_dsl.query import Q as ESDSLQ +from opaque_keys.edx.keys import CourseKey from parler.models import TranslatableModel, TranslatedFieldsModel from simple_history.models import HistoricalRecords from solo.models import SingletonModel @@ -1983,6 +1984,8 @@ def could_be_marketable(self): """ if not self.type.is_marketable: return False + if CourseKey.from_string(self.key).deprecated: # Old Mongo courses are not marketed + return False return not self.draft @property diff --git a/course_discovery/apps/course_metadata/tests/factories.py b/course_discovery/apps/course_metadata/tests/factories.py index c9eefca4c8..31f821895f 100644 --- a/course_discovery/apps/course_metadata/tests/factories.py +++ b/course_discovery/apps/course_metadata/tests/factories.py @@ -210,7 +210,7 @@ def course_run_types(self, create, extracted, **kwargs): class CourseFactory(SalesforceRecordFactory): uuid = factory.LazyFunction(uuid4) - key = FuzzyText(prefix='course-id/') + key = FuzzyText(prefix='course-id+') key_for_reruns = FuzzyText(prefix='OrgX+') title = FuzzyText(prefix="Test çօմɾʂҽ ") short_description = FuzzyText(prefix="Test çօմɾʂҽ short description") @@ -292,7 +292,7 @@ class Meta: class CourseRunFactory(SalesforceRecordFactory): status = CourseRunStatus.Published uuid = factory.LazyFunction(uuid4) - key = FuzzyText(prefix='course-run-id/', suffix='/fake') + key = FuzzyText(prefix='course-v1:org+', suffix='+fake') external_key = None course = factory.SubFactory(CourseFactory) title_override = None diff --git a/course_discovery/apps/course_metadata/tests/test_emails.py b/course_discovery/apps/course_metadata/tests/test_emails.py index fd7409f548..20aad8a0b3 100644 --- a/course_discovery/apps/course_metadata/tests/test_emails.py +++ b/course_discovery/apps/course_metadata/tests/test_emails.py @@ -114,7 +114,7 @@ def assertEmailNotSent(self, function, reason): emails.logger.name, 'INFO', StringComparison('Not sending notification email for template course_metadata/email/.* because ' + - reason), + re.escape(reason)), ) ) @@ -149,22 +149,23 @@ def test_send_email_for_internal_review(self): restricted_url = self.partner.lms_admin_url.rstrip('/') + '/embargo/restrictedcourse/' self.assertEmailSent( emails.send_email_for_internal_review, - f'^Review requested: {self.course_run.key} - {self.course_run.title}$', + f'^Review requested: {re.escape(self.course_run.key)} - {self.course_run.title}$', [self.pc], both_regexes=[ 'Dear %s,' % self.pc.full_name, - 'MyOrg has submitted %s for review.' % self.course_run.key, + 'MyOrg has submitted %s for review.' % re.escape(self.course_run.key), ], html_regexes=[ 'View this course run in Publisher to review the changes and mark it as reviewed.' % self.publisher_url, - 'This is a good time to review this course run in Studio.' % self.studio_url, + 'This is a good time to review this course run in Studio.' % + re.escape(self.studio_url), 'Visit the restricted course admin page to set embargo rules for this course, ' 'as needed.' % restricted_url, ], text_regexes=[ '\n\nPublisher page: %s\n' % self.publisher_url, - '\n\nStudio page: %s\n' % self.studio_url, + '\n\nStudio page: %s\n' % re.escape(self.studio_url), '\n\nRestricted Course admin: %s\n' % restricted_url, ], ) @@ -225,7 +226,7 @@ def test_send_email_for_go_live(self): **kwargs, ) self.assertEmailContains( - subject=f'^Published: {self.course_run.key} - {self.course_run.title}$', + subject=f'^Published: {re.escape(self.course_run.key)} - {self.course_run.title}$', to_users=[self.pc], index=1, **kwargs, diff --git a/course_discovery/apps/course_metadata/tests/test_lookups.py b/course_discovery/apps/course_metadata/tests/test_lookups.py index 465f85c6f8..85e0138dda 100644 --- a/course_discovery/apps/course_metadata/tests/test_lookups.py +++ b/course_discovery/apps/course_metadata/tests/test_lookups.py @@ -1,5 +1,5 @@ import json -from urllib.parse import quote +from urllib.parse import quote, urlencode import pytest from django.test import TestCase @@ -17,7 +17,7 @@ class TestAutocomplete: def assert_valid_query_result(self, client, path, query, expected_result): """ Asserts a query made against the given endpoint returns the expected result. """ - response = client.get(path + f'?q={query}') + response = client.get(path + f'?{urlencode({"q": query})}') data = json.loads(response.content.decode('utf-8')) assert len(data['results']) == 1 assert data['results'][0]['text'] == str(expected_result) diff --git a/course_discovery/apps/course_metadata/tests/test_models.py b/course_discovery/apps/course_metadata/tests/test_models.py index a1e645f290..5dd8ce0f32 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/course_discovery/apps/course_metadata/tests/test_models.py @@ -896,6 +896,17 @@ def test_could_be_marketable(self, draft, type_is_marketable, expected): course_run.save() assert mock_publish_obj.called == expected + @ddt.data( + ('old/mongo/key', False), + ('course-v1:modern+style+key', True), + ) + @ddt.unpack + def test_old_mongo_not_marketable(self, key, expected): + course_run = factories.CourseRunFactory.create(key=key) + factories.SeatFactory.create(course_run=course_run) + assert course_run.is_marketable == expected + assert course_run.could_be_marketable == expected + class CourseRunTestsThatNeedSetUp(OAuth2Mixin, TestCase): """