diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 70686f0353..e83630113f 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -76,7 +76,7 @@ from student.auth import has_course_author_access, has_studio_write_access, has_ from student.roles import ( CourseInstructorRole, CourseStaffRole, CourseCreatorRole, GlobalStaff, UserBasedRole ) -from util.course import get_lms_link_for_about_page +from util.course import get_link_for_about_page from util.date_utils import get_default_time_display from util.json_request import JsonResponse, JsonResponseBadRequest, expect_json from util.milestones_helpers import ( @@ -1000,7 +1000,7 @@ def settings_handler(request, course_key_string): settings_context = { 'context_course': course_module, 'course_locator': course_key, - 'lms_link_for_about_page': get_lms_link_for_about_page(course_key), + 'lms_link_for_about_page': get_link_for_about_page(course_key, request.user), 'course_image_url': course_image_url(course_module, 'course_image'), 'banner_image_url': course_image_url(course_module, 'banner_image'), 'video_thumbnail_image_url': course_image_url(course_module, 'video_thumbnail_image'), diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 46720b326d..96a39cc762 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -2,27 +2,30 @@ Utility methods related to course """ import logging -from django.conf import settings +from django.conf import settings from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.catalog.utils import get_run_marketing_url + log = logging.getLogger(__name__) -def get_lms_link_for_about_page(course_key): +def get_link_for_about_page(course_key, user, catalog_course_run=None): """ Returns the url to the course about page. """ assert isinstance(course_key, CourseKey) if settings.FEATURES.get('ENABLE_MKTG_SITE'): - # Root will be "https://www.edx.org". The complete URL will still not be exactly correct, - # but redirects exist from www.edx.org to get to the Drupal course about page URL. - about_base = settings.MKTG_URLS['ROOT'] - else: - about_base = settings.LMS_ROOT_URL + if catalog_course_run: + marketing_url = catalog_course_run.get('marketing_url') + else: + marketing_url = get_run_marketing_url(course_key, user) + if marketing_url: + return marketing_url return u"{about_base_url}/courses/{course_key}/about".format( - about_base_url=about_base, - course_key=course_key.to_deprecated_string() + about_base_url=settings.LMS_ROOT_URL, + course_key=unicode(course_key) ) diff --git a/common/djangoapps/util/tests/test_course.py b/common/djangoapps/util/tests/test_course.py index 491119546b..77fe7aaeeb 100644 --- a/common/djangoapps/util/tests/test_course.py +++ b/common/djangoapps/util/tests/test_course.py @@ -1,36 +1,73 @@ """ Tests for course utils. """ - -from django.test import TestCase, override_settings +from django.core.cache import cache +import httpretty import mock -from opaque_keys.edx.locations import SlashSeparatedCourseKey -from util.course import get_lms_link_for_about_page +from opaque_keys.edx.keys import CourseKey + +from openedx.core.djangoapps.catalog.tests import factories +from openedx.core.djangoapps.catalog.utils import CatalogCacheUtility +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from student.tests.factories import UserFactory +from util.course import get_link_for_about_page -class LmsLinksTestCase(TestCase): - """ Tests for LMS links. """ +@httpretty.activate +class CourseAboutLinkTestCase(CatalogIntegrationMixin, CacheIsolationTestCase): + """ + Tests for Course About link. + """ - def test_about_page(self): - """ Get URL for about page, no marketing site """ + ENABLED_CACHES = ['default'] + + def setUp(self): + super(CourseAboutLinkTestCase, self).setUp() + self.user = UserFactory.create(password="password") + + self.course_key_string = "foo/bar/baz" + self.course_key = CourseKey.from_string("foo/bar/baz") + self.course_run = factories.CourseRun(key=self.course_key_string) + self.lms_course_about_url = "http://localhost:8000/courses/foo/bar/baz/about" + + self.catalog_integration = self.create_catalog_integration( + internal_api_url="http://catalog.example.com:443/api/v1", + cache_ttl=1 + ) + self.course_cache_key = "{}{}".format(CatalogCacheUtility.CACHE_KEY_PREFIX, self.course_key_string) + + def test_about_page_lms(self): + """ + Get URL for about page, no marketing site. + """ with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - self.assertEquals(self.get_about_page_link(), "http://localhost:8000/courses/mitX/101/test/about") + self.assertEquals( + get_link_for_about_page(self.course_key, self.user), self.lms_course_about_url + ) + with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): + self.register_catalog_course_run_response( + [self.course_key_string], [{"key": self.course_key_string, "marketing_url": None}] + ) + self.assertEquals(get_link_for_about_page(self.course_key, self.user), self.lms_course_about_url) - @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root'}) + @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) def test_about_page_marketing_site(self): - """ Get URL for about page, marketing root present. """ - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - self.assertEquals(self.get_about_page_link(), "https://dummy-root/courses/mitX/101/test/about") - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - self.assertEquals(self.get_about_page_link(), "http://localhost:8000/courses/mitX/101/test/about") + """ + Get URL for about page, marketing site enabled. + """ + self.register_catalog_course_run_response([self.course_key_string], [self.course_run]) + self.assertEquals(get_link_for_about_page(self.course_key, self.user), self.course_run["marketing_url"]) + cached_data = cache.get_many([self.course_cache_key]) + self.assertIn(self.course_cache_key, cached_data.keys()) - @override_settings(MKTG_URLS={'ROOT': 'https://www.dummyhttps://x'}) - def test_about_page_marketing_site_https__edge(self): - """ Get URL for about page """ - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - self.assertEquals(self.get_about_page_link(), "https://www.dummyhttps://x/courses/mitX/101/test/about") + with mock.patch('openedx.core.djangoapps.catalog.utils.get_edx_api_data') as mock_method: + self.assertEquals(get_link_for_about_page(self.course_key, self.user), self.course_run["marketing_url"]) + self.assertEqual(0, mock_method.call_count) - def get_about_page_link(self): - """ create mock course and return the about page link.""" - course_key = SlashSeparatedCourseKey('mitX', '101', 'test') - return get_lms_link_for_about_page(course_key) + @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) + def test_about_page_marketing_url_cached(self): + self.assertEquals( + get_link_for_about_page(self.course_key, self.user, self.course_run), + self.course_run["marketing_url"] + ) diff --git a/lms/djangoapps/learner_dashboard/tests/test_programs.py b/lms/djangoapps/learner_dashboard/tests/test_programs.py index cd4a46743f..0a2ec75101 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_programs.py +++ b/lms/djangoapps/learner_dashboard/tests/test_programs.py @@ -17,6 +17,8 @@ import httpretty import mock from provider.constants import CONFIDENTIAL +from openedx.core.djangoapps.catalog.tests import factories as catalog_factories +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.tests import factories as credentials_factories from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin @@ -265,9 +267,10 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar @httpretty.activate @override_settings(MKTG_URLS={'ROOT': 'https://www.example.com'}) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -@mock.patch(UTILS_MODULE + '.get_run_marketing_url', mock.Mock(return_value=MARKETING_URL)) -class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): - """Unit tests for the program details page.""" +class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, CatalogIntegrationMixin): + """ + Unit tests for the program details page. + """ program_id = 123 password = 'test' url = reverse('program_details_view', args=[program_id]) @@ -279,6 +282,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) course = CourseFactory() + cls.course_id_string = unicode(course.id) # pylint: disable=no-member organization = programs_factories.Organization() run_mode = programs_factories.RunMode(course_key=unicode(course.id)) # pylint: disable=no-member course_code = programs_factories.CourseCode(run_modes=[run_mode]) @@ -287,6 +291,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): organizations=[organization], course_codes=[course_code] ) + cls.course_run = catalog_factories.CourseRun(key=cls.course_id_string) def setUp(self): super(TestProgramDetails, self).setUp() @@ -295,7 +300,9 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): self.client.login(username=self.user.username, password=self.password) def mock_programs_api(self, data, status=200): - """Helper for mocking out Programs API URLs.""" + """ + Helper for mocking out Programs API URLs. + """ self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') url = '{api_root}/programs/{id}/'.format( @@ -314,7 +321,9 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): ) def assert_program_data_present(self, response): - """Verify that program data is present.""" + """ + Verify that program data is present. + """ self.assertContains(response, 'programData') self.assertContains(response, 'urls') self.assertContains(response, 'program_listing_url') @@ -322,7 +331,9 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): self.assert_programs_tab_present(response) def assert_programs_tab_present(self, response): - """Verify that the programs tab is present in the nav.""" + """ + Verify that the programs tab is present in the nav. + """ soup = BeautifulSoup(response.content, 'html.parser') self.assertTrue( any(soup.find_all('a', class_='tab-nav-link', href=reverse('program_listing_view'))) @@ -332,21 +343,24 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): """ Verify that login is required to access the page. """ - self.create_programs_config() - self.mock_programs_api(self.data) + with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ + self.course_id_string: self.course_run, + }): + self.create_programs_config() + self.mock_programs_api(self.data) - self.client.logout() + self.client.logout() - response = self.client.get(self.url) - self.assertRedirects( - response, - '{}?next={}'.format(reverse('signin_user'), self.url) - ) + response = self.client.get(self.url) + self.assertRedirects( + response, + '{}?next={}'.format(reverse('signin_user'), self.url) + ) - self.client.login(username=self.user.username, password=self.password) + self.client.login(username=self.user.username, password=self.password) - response = self.client.get(self.url) - self.assert_program_data_present(response) + response = self.client.get(self.url) + self.assert_program_data_present(response) def test_404_if_disabled(self): """ @@ -358,7 +372,9 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): self.assertEqual(response.status_code, 404) def test_404_if_no_data(self): - """Verify that the page 404s if no program data is found.""" + """ + Verify that the page 404s if no program data is found. + """ self.create_programs_config() self.mock_programs_api(self.data, status=404) @@ -372,7 +388,9 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): self.assertEqual(response.status_code, 404) def test_page_routing(self): - """Verify that the page can be hit with or without a program name in the URL.""" + """ + Verify that the page can be hit with or without a program name in the URL. + """ self.create_programs_config() self.mock_programs_api(self.data) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index e759f358e5..5f734f2bf1 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -1,14 +1,12 @@ """ Serializer for user API """ -from opaque_keys.edx.keys import CourseKey from rest_framework import serializers from rest_framework.reverse import reverse - from courseware.access import has_access -from student.models import CourseEnrollment, User from certificates.api import certificate_downloadable_status -from util.course import get_lms_link_for_about_page +from student.models import CourseEnrollment, User +from util.course import get_link_for_about_page class CourseOverviewField(serializers.RelatedField): @@ -52,7 +50,9 @@ class CourseOverviewField(serializers.RelatedField): } }, 'course_image': course_overview.course_image_url, - 'course_about': get_lms_link_for_about_page(CourseKey.from_string(course_id)), + 'course_about': get_link_for_about_page( + course_overview.id, request.user, self.context.get('catalog_course_run') + ), 'course_updates': reverse( 'course-updates-list', kwargs={'course_id': course_id}, @@ -85,7 +85,9 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): certificate = serializers.SerializerMethodField() def get_certificate(self, model): - """Returns the information about the user's certificate in the course.""" + """ + Returns the information about the user's certificate in the course. + """ certificate_info = certificate_downloadable_status(model.user, model.course_id) if certificate_info['is_downloadable']: return { diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index da40f21ca0..091367411b 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -5,7 +5,9 @@ Tests for users API import datetime import ddt +import httpretty from mock import patch +import mock from nose.plugins.attrib import attr import pytz from django.conf import settings @@ -26,6 +28,9 @@ from courseware.access_response import ( ) from course_modes.models import CourseMode from lms.djangoapps.grades.tests.utils import mock_passing_grade +from openedx.core.djangoapps.catalog.tests import factories +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin +from openedx.core.djangoapps.catalog.utils import get_course_runs from openedx.core.lib.courses import course_image_url from student.models import CourseEnrollment from util.milestones_helpers import set_prerequisite_courses @@ -463,7 +468,8 @@ class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, @attr(shard=2) @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) -class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin): +@httpretty.activate +class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin, CatalogIntegrationMixin): """ Test the course enrollment serializer """ @@ -472,6 +478,13 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin) self.login_and_enroll() self.request = RequestFactory().get('/') self.request.user = self.user + self.catalog_integration = self.create_catalog_integration( + internal_api_url="http://catalog.example.com:443/api/v1", + cache_ttl=1 + ) + self.course_id_string = unicode(self.course.id) + self.course_run = factories.CourseRun(key=self.course_id_string) + self.course_keys = [self.course_id_string] def test_success(self): serialized = CourseEnrollmentSerializer( @@ -493,3 +506,41 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin) ).data self.assertEqual(serialized['course']['number'], self.course.display_coursenumber) self.assertEqual(serialized['course']['org'], self.course.display_organization) + + @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) + def test_course_about_marketing_url(self): + self.register_catalog_course_run_response(self.course_keys, [self.course_run]) + catalog_course_runs_against_course_keys = get_course_runs(self.course_keys, self.request.user) + enrollment = CourseEnrollment.enrollments_for_user(self.user)[0] + serialized = CourseEnrollmentSerializer( + enrollment, + context={ + 'request': self.request, + "catalog_course_run": ( + catalog_course_runs_against_course_keys[unicode(enrollment.course_id)] + if unicode(enrollment.course_id) in catalog_course_runs_against_course_keys else None + ) + }, + ).data + self.assertEqual( + serialized['course']['course_about'], self.course_run["marketing_url"] + ) + + @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}) + def test_course_about_lms_url(self): + self.register_catalog_course_run_response(self.course_keys, [self.course_run]) + catalog_course_runs_against_course_keys = get_course_runs(self.course_keys, self.request.user) + enrollment = CourseEnrollment.enrollments_for_user(self.user)[0] + serialized = CourseEnrollmentSerializer( + enrollment, + context={ + 'request': self.request, + "catalog_course_run": ( + catalog_course_runs_against_course_keys[unicode(enrollment.course_id)] + if unicode(enrollment.course_id) in catalog_course_runs_against_course_keys else None + ) + }, + ).data + self.assertEqual( + serialized['course']['course_about'], "http://localhost:8000/courses/{}/about".format(self.course_id_string) + ) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 3d8b4fba51..e63a737f48 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -11,12 +11,14 @@ from rest_framework.response import Response from opaque_keys.edx.keys import UsageKey from opaque_keys import InvalidKeyError +from rest_framework.views import APIView from courseware.access import is_mobile_available_for_user from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from courseware.views.index import save_positions_recursively_up from courseware.views.views import get_current_child +from openedx.core.djangoapps.catalog.utils import get_course_runs from student.models import CourseEnrollment, User from xblock.fields import Scope @@ -204,7 +206,7 @@ class UserCourseStatus(views.APIView): @mobile_view(is_user=True) -class UserCourseEnrollmentsList(generics.ListAPIView): +class UserCourseEnrollmentsList(APIView): """ **Use Case** @@ -258,17 +260,6 @@ class UserCourseEnrollmentsList(generics.ListAPIView): certified). * url: URL to the downloadable version of the certificate, if exists. """ - queryset = CourseEnrollment.objects.all() - serializer_class = CourseEnrollmentSerializer - lookup_field = 'username' - - # In Django Rest Framework v3, there is a default pagination - # class that transmutes the response data into a dictionary - # with pagination information. The original response data (a list) - # is stored in a "results" value of the dictionary. - # For backwards compatibility with the existing API, we disable - # the default behavior by setting the pagination_class to None. - pagination_class = None def is_org(self, check_org, course_org): """ @@ -276,17 +267,34 @@ class UserCourseEnrollmentsList(generics.ListAPIView): """ return check_org is None or (check_org.lower() == course_org.lower()) - def get_queryset(self): - enrollments = self.queryset.filter( - user__username=self.kwargs['username'], + def get(self, request, username): + """ + Returns a list of courses enrolled by user. + """ + queryset = CourseEnrollment.objects.all() + course_ids = set(queryset.values_list('course_id', flat=True)) + catalog_course_runs_against_course_keys = get_course_runs(course_ids, request.user) + + enrollments = queryset.filter( + user__username=username, is_active=True ).order_by('created').reverse() - org = self.request.query_params.get('org', None) - return [ - enrollment for enrollment in enrollments + org = request.query_params.get('org', None) + + return Response([ + CourseEnrollmentSerializer( + enrollment, + context={ + "request": request, + "catalog_course_run": ( + catalog_course_runs_against_course_keys[unicode(enrollment.course_id)] + if unicode(enrollment.course_id) in catalog_course_runs_against_course_keys else None + ) + } + ).data for enrollment in enrollments if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) and - is_mobile_available_for_user(self.request.user, enrollment.course_overview) - ] + is_mobile_available_for_user(request.user, enrollment.course_overview) + ]) @api_view(["GET"]) diff --git a/openedx/core/djangoapps/catalog/tests/mixins.py b/openedx/core/djangoapps/catalog/tests/mixins.py index 2372a0b33f..65886373c1 100644 --- a/openedx/core/djangoapps/catalog/tests/mixins.py +++ b/openedx/core/djangoapps/catalog/tests/mixins.py @@ -1,9 +1,16 @@ """Mixins to help test catalog integration.""" +import json +import urllib + +import httpretty + from openedx.core.djangoapps.catalog.models import CatalogIntegration class CatalogIntegrationMixin(object): - """Utility for working with the catalog service during testing.""" + """ + Utility for working with the catalog service during testing. + """ DEFAULTS = { 'enabled': True, @@ -12,8 +19,27 @@ class CatalogIntegrationMixin(object): } def create_catalog_integration(self, **kwargs): - """Creates a new CatalogIntegration with DEFAULTS, updated with any provided overrides.""" + """ + Creates a new CatalogIntegration with DEFAULTS, updated with any provided overrides. + """ fields = dict(self.DEFAULTS, **kwargs) CatalogIntegration(**fields).save() return CatalogIntegration.current() + + def register_catalog_course_run_response(self, course_keys, catalog_course_run_data): + """ + Register a mock response for GET on the catalog course run endpoint. + """ + httpretty.register_uri( + httpretty.GET, + "http://catalog.example.com:443/api/v1/course_runs/?keys={}&exclude_utm=1".format( + urllib.quote_plus(",".join(course_keys)) + ), + body=json.dumps({ + "results": catalog_course_run_data, + "next": "" + }), + content_type='application/json', + status=200 + ) diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index b43694e415..bb813ba277 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -1,14 +1,18 @@ -"""Tests covering utilities for integrating with the catalog service.""" +""" +Tests covering utilities for integrating with the catalog service. +""" import uuid -import ddt +from django.core.cache import cache from django.test import TestCase +import httpretty import mock from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.catalog import utils from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.catalog.tests import factories, mixins +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.tests.factories import UserFactory @@ -19,7 +23,9 @@ UTILS_MODULE = 'openedx.core.djangoapps.catalog.utils' # ConfigurationModels use the cache. Make every cache get a miss. @mock.patch('config_models.models.cache.get', return_value=None) class TestGetPrograms(mixins.CatalogIntegrationMixin, TestCase): - """Tests covering retrieval of programs from the catalog service.""" + """ + Tests covering retrieval of programs from the catalog service. + """ def setUp(self): super(TestGetPrograms, self).setUp() @@ -29,7 +35,9 @@ class TestGetPrograms(mixins.CatalogIntegrationMixin, TestCase): self.catalog_integration = self.create_catalog_integration(cache_ttl=1) def assert_contract(self, call_args, program_uuid=None, type=None): # pylint: disable=redefined-builtin - """Verify that API data retrieval utility is used correctly.""" + """ + Verify that API data retrieval utility is used correctly. + """ args, kwargs = call_args for arg in (self.catalog_integration, self.user, 'programs'): @@ -108,7 +116,9 @@ class TestGetPrograms(mixins.CatalogIntegrationMixin, TestCase): class TestMungeCatalogProgram(TestCase): - """Tests covering querystring stripping.""" + """ + Tests covering querystring stripping. + """ catalog_program = factories.Program() def test_munge_catalog_program(self): @@ -153,90 +163,173 @@ class TestMungeCatalogProgram(TestCase): self.assertEqual(munged, expected) -@mock.patch(UTILS_MODULE + '.get_edx_api_data') -@mock.patch('config_models.models.cache.get', return_value=None) -class TestGetCourseRun(mixins.CatalogIntegrationMixin, TestCase): - """Tests covering retrieval of course runs from the catalog service.""" +@httpretty.activate +class TestGetCourseRun(mixins.CatalogIntegrationMixin, CacheIsolationTestCase): + """ + Tests covering retrieval of course runs from the catalog service. + """ + + ENABLED_CACHES = ['default'] + def setUp(self): super(TestGetCourseRun, self).setUp() self.user = UserFactory() - self.course_key = CourseKey.from_string('foo/bar/baz') - self.catalog_integration = self.create_catalog_integration() + self.catalog_integration = self.create_catalog_integration( + internal_api_url="http://catalog.example.com:443/api/v1", + cache_ttl=1, + ) + self.course_runs = [factories.CourseRun() for __ in range(4)] + self.course_key_1 = CourseKey.from_string(self.course_runs[0]["key"]) + self.course_key_2 = CourseKey.from_string(self.course_runs[1]["key"]) + self.course_key_3 = CourseKey.from_string(self.course_runs[2]["key"]) + self.course_key_4 = CourseKey.from_string(self.course_runs[3]["key"]) - def assert_contract(self, call_args): - """Verify that API data retrieval utility is used correctly.""" - args, kwargs = call_args - - for arg in (self.catalog_integration, self.user, 'course_runs'): - self.assertIn(arg, args) - - self.assertEqual(kwargs['resource_id'], unicode(self.course_key)) - self.assertEqual(kwargs['api']._store['base_url'], self.catalog_integration.internal_api_url) # pylint: disable=protected-access - - return args, kwargs - - def test_get_course_run(self, _mock_cache, mock_get_catalog_data): - course_run = factories.CourseRun() - mock_get_catalog_data.return_value = course_run - - data = utils.get_course_run(self.course_key, self.user) - - self.assert_contract(mock_get_catalog_data.call_args) - self.assertEqual(data, course_run) - - def test_course_run_unavailable(self, _mock_cache, mock_get_catalog_data): - mock_get_catalog_data.return_value = [] - - data = utils.get_course_run(self.course_key, self.user) - - self.assert_contract(mock_get_catalog_data.call_args) - self.assertEqual(data, {}) - - def test_cache_disabled(self, _mock_cache, mock_get_catalog_data): - utils.get_course_run(self.course_key, self.user) - - _, kwargs = self.assert_contract(mock_get_catalog_data.call_args) - - self.assertIsNone(kwargs['cache_key']) - - def test_cache_enabled(self, _mock_cache, mock_get_catalog_data): - catalog_integration = self.create_catalog_integration(cache_ttl=1) - - utils.get_course_run(self.course_key, self.user) - - _, kwargs = mock_get_catalog_data.call_args - - self.assertEqual(kwargs['cache_key'], catalog_integration.CACHE_KEY) - - def test_config_missing(self, _mock_cache, _mock_get_catalog_data): - """Verify that no errors occur if this method is called when catalog config is missing.""" + def test_config_missing(self): + """ + Verify that no errors occur if this method is called when catalog config is missing. + """ CatalogIntegration.objects.all().delete() - data = utils.get_course_run(self.course_key, self.user) + data = utils.get_course_runs([], self.user) self.assertEqual(data, {}) + def test_get_course_run(self): + course_keys = [self.course_key_1] + course_key_strings = [self.course_runs[0]["key"]] + self.register_catalog_course_run_response(course_key_strings, [self.course_runs[0]]) -@mock.patch(UTILS_MODULE + '.get_course_run') -class TestGetRunMarketingUrl(TestCase): - """Tests covering retrieval of course run marketing URLs.""" + course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) + expected_data = {self.course_runs[0]["key"]: self.course_runs[0]} + self.assertEqual(expected_data, course_catalog_data_dict) + + def test_get_multiple_course_run(self): + course_key_strings = [self.course_runs[0]["key"], self.course_runs[1]["key"], self.course_runs[2]["key"]] + course_keys = [self.course_key_1, self.course_key_2, self.course_key_3] + self.register_catalog_course_run_response( + course_key_strings, [self.course_runs[0], self.course_runs[1], self.course_runs[2]] + ) + + course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) + expected_data = { + self.course_runs[0]["key"]: self.course_runs[0], + self.course_runs[1]["key"]: self.course_runs[1], + self.course_runs[2]["key"]: self.course_runs[2], + } + self.assertEqual(expected_data, course_catalog_data_dict) + + def test_course_run_unavailable(self): + course_key_strings = [self.course_runs[0]["key"], self.course_runs[3]["key"]] + course_keys = [self.course_key_1, self.course_key_4] + self.register_catalog_course_run_response(course_key_strings, [self.course_runs[0]]) + + course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) + expected_data = {self.course_runs[0]["key"]: self.course_runs[0]} + self.assertEqual(expected_data, course_catalog_data_dict) + + def test_cached_course_run_data(self): + course_key_strings = [self.course_runs[0]["key"], self.course_runs[1]["key"]] + course_keys = [self.course_key_1, self.course_key_2] + course_cached_keys = [ + "{}{}".format(utils.CatalogCacheUtility.CACHE_KEY_PREFIX, self.course_runs[0]["key"]), + "{}{}".format(utils.CatalogCacheUtility.CACHE_KEY_PREFIX, self.course_runs[1]["key"]), + ] + self.register_catalog_course_run_response(course_key_strings, [self.course_runs[0], self.course_runs[1]]) + expected_data = { + self.course_runs[0]["key"]: self.course_runs[0], + self.course_runs[1]["key"]: self.course_runs[1], + } + + course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) + self.assertEqual(expected_data, course_catalog_data_dict) + cached_data = cache.get_many(course_cached_keys) + self.assertEqual(set(course_cached_keys), set(cached_data.keys())) + + with mock.patch('openedx.core.djangoapps.catalog.utils.get_edx_api_data') as mock_method: + course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) + self.assertEqual(0, mock_method.call_count) + self.assertEqual(expected_data, course_catalog_data_dict) + + +class TestGetRunMarketingUrl(TestCase, mixins.CatalogIntegrationMixin): + """ + Tests covering retrieval of course run marketing URLs. + """ def setUp(self): super(TestGetRunMarketingUrl, self).setUp() - - self.course_key = CourseKey.from_string('foo/bar/baz') self.user = UserFactory() + self.course_runs = [factories.CourseRun() for __ in range(2)] + self.course_key_1 = CourseKey.from_string(self.course_runs[0]["key"]) - def test_get_run_marketing_url(self, mock_get_course_run): - course_run = factories.CourseRun() - mock_get_course_run.return_value = course_run + def test_get_run_marketing_url(self): + with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ + self.course_runs[0]["key"]: self.course_runs[0], + self.course_runs[1]["key"]: self.course_runs[1], + }): + course_marketing_url = utils.get_run_marketing_url(self.course_key_1, self.user) + self.assertEqual(self.course_runs[0]["marketing_url"], course_marketing_url) - url = utils.get_run_marketing_url(self.course_key, self.user) + def test_marketing_url_catalog_course_run_not_found(self): + with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ + self.course_runs[0]["key"]: self.course_runs[0], + }): + course_marketing_url = utils.get_run_marketing_url(self.course_key_1, self.user) + self.assertEqual(self.course_runs[0]["marketing_url"], course_marketing_url) - self.assertEqual(url, course_run['marketing_url']) + def test_marketing_url_missing(self): + self.course_runs[1]["marketing_url"] = None + with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ + self.course_runs[0]["key"]: self.course_runs[0], + self.course_runs[1]["key"]: self.course_runs[1], + }): + course_marketing_url = utils.get_run_marketing_url(CourseKey.from_string("foo2/bar2/baz2"), self.user) + self.assertEqual(None, course_marketing_url) - def test_marketing_url_missing(self, mock_get_course_run): - mock_get_course_run.return_value = {} - url = utils.get_run_marketing_url(self.course_key, self.user) +class TestGetRunMarketingUrls(TestCase, mixins.CatalogIntegrationMixin): + """ + Tests covering retrieval of course run marketing URLs. + """ + def setUp(self): + super(TestGetRunMarketingUrls, self).setUp() + self.user = UserFactory() + self.course_runs = [factories.CourseRun() for __ in range(2)] + self.course_keys = [ + CourseKey.from_string(self.course_runs[0]["key"]), + CourseKey.from_string(self.course_runs[1]["key"]), + ] - self.assertEqual(url, None) + def test_get_run_marketing_url(self): + expected_data = { + self.course_runs[0]["key"]: self.course_runs[0]["marketing_url"], + self.course_runs[1]["key"]: self.course_runs[1]["marketing_url"], + } + with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ + self.course_runs[0]["key"]: self.course_runs[0], + self.course_runs[1]["key"]: self.course_runs[1], + }): + course_marketing_url_dict = utils.get_run_marketing_urls(self.course_keys, self.user) + self.assertEqual(expected_data, course_marketing_url_dict) + + def test_marketing_url_catalog_course_run_not_found(self): + expected_data = { + self.course_runs[0]["key"]: self.course_runs[0]["marketing_url"], + } + with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ + self.course_runs[0]["key"]: self.course_runs[0], + }): + course_marketing_url_dict = utils.get_run_marketing_urls(self.course_keys, self.user) + self.assertEqual(expected_data, course_marketing_url_dict) + + def test_marketing_url_missing(self): + expected_data = { + self.course_runs[0]["key"]: self.course_runs[0]["marketing_url"], + self.course_runs[1]["key"]: None, + } + self.course_runs[1]["marketing_url"] = None + with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ + self.course_runs[0]["key"]: self.course_runs[0], + self.course_runs[1]["key"]: self.course_runs[1], + }): + course_marketing_url_dict = utils.get_run_marketing_urls(self.course_keys, self.user) + self.assertEqual(expected_data, course_marketing_url_dict) diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index 7353c78403..44bb8f36cb 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -1,7 +1,9 @@ """Helper functions for working with the catalog service.""" -from urlparse import urlparse +import abc +import logging from django.conf import settings +from django.core.cache import cache from edx_rest_api_client.client import EdxRestApiClient from opaque_keys.edx.keys import CourseKey @@ -10,6 +12,9 @@ from openedx.core.lib.edx_api_utils import get_edx_api_data from openedx.core.lib.token_utils import JwtBuilder +log = logging.getLogger(__name__) + + def create_catalog_api_client(user, catalog_integration): """Returns an API client which can be used to make catalog API requests.""" scopes = ['email', 'profile'] @@ -115,38 +120,57 @@ def munge_catalog_program(catalog_program): } -def get_course_run(course_key, user): - """Get a course run's data from the course catalog service. - - Arguments: - course_key (CourseKey): Course key object identifying the run whose data we want. - user (User): The user to authenticate as when making requests to the catalog service. - - Returns: - dict, empty if no data could be retrieved. +def get_and_cache_course_runs(course_keys, user): """ + Get course run's data from the course catalog service and cache it. + """ + catalog_course_runs_against_course_keys = {} catalog_integration = CatalogIntegration.current() - if catalog_integration.enabled: api = create_catalog_api_client(user, catalog_integration) - data = get_edx_api_data( + catalog_data = get_edx_api_data( catalog_integration, user, 'course_runs', - resource_id=unicode(course_key), - cache_key=catalog_integration.CACHE_KEY if catalog_integration.is_cache_enabled else None, api=api, - querystring={'exclude_utm': 1}, + querystring={'keys': ",".join(course_keys), 'exclude_utm': 1}, + ) + if catalog_data: + for catalog_course_run in catalog_data: + CatalogCacheUtility.cache_course_run(catalog_course_run) + catalog_course_runs_against_course_keys[catalog_course_run["key"]] = catalog_course_run + return catalog_course_runs_against_course_keys + + +def get_course_runs(course_keys, user): + """ + Get course run data from the course catalog service if not available in cache. + + Arguments: + course_keys ([CourseKey]): A list of Course key object identifying the run whose data we want. + user (User): The user to authenticate as when making requests to the catalog service. + + Returns: + dict of catalog course runs against course keys, empty if no data could be retrieved. + """ + catalog_course_runs_against_course_keys = CatalogCacheUtility.get_cached_catalog_course_runs( + course_keys + ) + if len(catalog_course_runs_against_course_keys.keys()) != len(course_keys): + missing_course_keys = CatalogCacheUtility.get_course_keys_not_found_in_cache( + course_keys, catalog_course_runs_against_course_keys.keys() + ) + catalog_course_runs_against_course_keys.update( + get_and_cache_course_runs(missing_course_keys, user) ) - return data if data else {} - else: - return {} + return catalog_course_runs_against_course_keys def get_run_marketing_url(course_key, user): - """Get a course run's marketing URL from the course catalog service. + """ + Get a course run's marketing URL from the course catalog service. Arguments: course_key (CourseKey): Course key object identifying the run whose marketing URL we want. @@ -155,5 +179,111 @@ def get_run_marketing_url(course_key, user): Returns: string, the marketing URL, or None if no URL is available. """ - course_run = get_course_run(course_key, user) - return course_run.get('marketing_url') + course_marketing_urls = get_run_marketing_urls([course_key], user) + return course_marketing_urls.get(unicode(course_key)) + + +def get_run_marketing_urls(course_keys, user): + """ + Get course run marketing URLs from the course catalog service against course keys. + + Arguments: + course_keys ([CourseKey]): A list of Course key object identifying the run whose data we want. + user (User): The user to authenticate as when making requests to the catalog service. + + Returns: + dict of run marketing URLs against course keys + """ + course_marketing_urls = {} + course_catalog_dict = get_course_runs(course_keys, user) + if not course_catalog_dict: + return course_marketing_urls + + for course_key in course_keys: + course_key_string = unicode(course_key) + if course_key_string in course_catalog_dict: + course_marketing_urls[course_key_string] = course_catalog_dict[course_key_string].get('marketing_url') + + return course_marketing_urls + + +class CatalogCacheUtility(object): + """ + Non-instantiatable class housing utility methods for caching catalog API data. + """ + __metaclass__ = abc.ABCMeta + CACHE_KEY_PREFIX = "catalog.course_runs." + + @classmethod + def get_course_keys_not_found_in_cache(cls, course_keys, cached_course_run_keys): + """ + Get course key strings for which course run data is not available in cache. + + Arguments: + course_key (CourseKey): CourseKey object to create corresponding catalog cache key. + + Returns: + list of string rep of course keys not found in catalog cache. + """ + missing_course_keys = [] + for course_key in course_keys: + course_key_string = unicode(course_key) + if course_key_string not in cached_course_run_keys: + missing_course_keys.append(course_key_string) + + log.info("Cached catalog course run data missing for: '{}'".format( + ", ".join(missing_course_keys) + )) + return missing_course_keys + + @classmethod + def get_cached_catalog_course_runs(cls, course_keys): + """ + Get course runs from cache against course keys. + + Arguments: + course_keys ([CourseKey]): List of CourseKey object identifying the run whose data we want. + + Returns: + dict of catalog course run against course key string + """ + course_catalog_run_cache_keys = [ + cls._get_cache_key_name(course_key) + for course_key in course_keys + ] + cached_catalog_course_runs = cache.get_many(course_catalog_run_cache_keys) + return { + cls._extract_course_key_from_cache_key_name(cached_key): cached_course_run + for cached_key, cached_course_run in cached_catalog_course_runs.iteritems() + } + + @classmethod + def cache_course_run(cls, catalog_course_run): + """ + Caches catalog course run for course key. + """ + cache.set( + cls._get_cache_key_name(catalog_course_run["key"]), + catalog_course_run, + CatalogIntegration.current().cache_ttl + ) + + @classmethod + def _get_cache_key_name(cls, course_key): + """ + Returns key name to use to cache catalog course run data for course key. + + Arguments: + course_key (CourseKey): CourseKey object to create corresponding catalog cache key. + + Returns: + string, catalog cache key against course key. + """ + return "{}{}".format(cls.CACHE_KEY_PREFIX, unicode(course_key)) + + @classmethod + def _extract_course_key_from_cache_key_name(cls, catalog_course_run_cache_key): + """ + Returns course_key extracted from cache key of catalog course run data. + """ + return catalog_course_run_cache_key.replace(cls.CACHE_KEY_PREFIX, '') diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index b482cbc1b3..b546b1d6ee 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -13,15 +13,16 @@ from django.test import TestCase from django.test.utils import override_settings from django.utils import timezone from django.utils.text import slugify +from edx_oauth2_provider.tests.factories import ClientFactory import httpretty import mock from nose.plugins.attrib import attr from opaque_keys.edx.keys import CourseKey -from edx_oauth2_provider.tests.factories import ClientFactory from provider.constants import CONFIDENTIAL from lms.djangoapps.certificates.api import MODES from lms.djangoapps.commerce.tests.test_utils import update_commerce_config +from openedx.core.djangoapps.catalog.tests import factories as catalog_factories from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.credentials.tests import factories as credentials_factories from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin, CredentialsDataMixin @@ -703,7 +704,9 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @mock.patch(UTILS_MODULE + '.get_run_marketing_url', mock.Mock(return_value=MARKETING_URL)) class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): - """Tests of the program data extender utility class.""" + """ + Tests of the program data extender utility class. + """ maxDiff = None sku = 'abc123' password = 'test' @@ -721,6 +724,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): self.course.start = timezone.now() - datetime.timedelta(days=1) self.course.end = timezone.now() + datetime.timedelta(days=1) self.course = self.update_course(self.course, self.user.id) # pylint: disable=no-member + self.course_id_string = unicode(self.course.id) # pylint: disable=no-member self.organization = factories.Organization() self.run_mode = factories.RunMode(course_key=unicode(self.course.id)) # pylint: disable=no-member @@ -729,9 +733,12 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): organizations=[self.organization], course_codes=[self.course_code] ) + self.course_run = catalog_factories.CourseRun(key=self.course_id_string) def _assert_supplemented(self, actual, **kwargs): - """DRY helper used to verify that program data is extended correctly.""" + """ + DRY helper used to verify that program data is extended correctly. + """ course_overview = CourseOverview.get_from_id(self.course.id) # pylint: disable=no-member run_mode = dict( factories.RunMode( @@ -764,7 +771,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @ddt.unpack @mock.patch(UTILS_MODULE + '.CourseMode.mode_for_course') def test_student_enrollment_status(self, is_enrolled, enrolled_mode, is_upgrade_required, mock_get_mode): - """Verify that program data is supplemented with the student's enrollment status.""" + """ + Verify that program data is supplemented with the student's enrollment status. + """ expected_upgrade_url = '{root}/{path}?sku={sku}'.format( root=ECOMMERCE_URL_ROOT, path=self.checkout_path.strip('/'), @@ -790,7 +799,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @ddt.data(MODES.audit, MODES.verified) def test_inactive_enrollment_no_upgrade(self, enrolled_mode): - """Verify that a student with an inactive enrollment isn't encouraged to upgrade.""" + """ + Verify that a student with an inactive enrollment isn't encouraged to upgrade. + """ update_commerce_config(enabled=True, checkout_page=self.checkout_path) CourseEnrollmentFactory( @@ -806,7 +817,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.CourseMode.mode_for_course') def test_ecommerce_disabled(self, mock_get_mode): - """Verify that the utility can operate when the ecommerce service is disabled.""" + """ + Verify that the utility can operate when the ecommerce service is disabled. + """ update_commerce_config(enabled=False, checkout_page=self.checkout_path) mock_mode = mock.Mock() @@ -825,7 +838,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): ) @ddt.unpack def test_course_enrollment_status(self, start_offset, end_offset, is_enrollment_open): - """Verify that course enrollment status is reflected correctly.""" + """ + Verify that course enrollment status is reflected correctly. + """ self.course.enrollment_start = timezone.now() - datetime.timedelta(days=start_offset) self.course.enrollment_end = timezone.now() - datetime.timedelta(days=end_offset) self.course = self.update_course(self.course, self.user.id) # pylint: disable=no-member @@ -839,7 +854,8 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): ) def test_no_enrollment_start_date(self): - """Verify that a closed course with no explicit enrollment start date doesn't cause an error. + """ + Verify that a closed course with no explicit enrollment start date doesn't cause an error. Regression test for ECOM-4973. """ @@ -857,7 +873,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.certificate_api.certificate_downloadable_status') @mock.patch(CERTIFICATES_API_MODULE + '.has_html_certificates_enabled') def test_certificate_url_retrieval(self, is_uuid_available, mock_html_certs_enabled, mock_get_cert_data): - """Verify that the student's run mode certificate is included, when available.""" + """ + Verify that the student's run mode certificate is included, when available. + """ test_uuid = uuid.uuid4().hex mock_get_cert_data.return_value = {'uuid': test_uuid} if is_uuid_available else {} mock_html_certs_enabled.return_value = True @@ -882,7 +900,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.get_organization_by_short_name') def test_organization_logo_exists(self, mock_get_organization_by_short_name): - """ Verify the logo image is set from the organizations api """ + """ + Verify the logo image is set from the organizations api. + """ mock_logo_url = 'edx/logo.png' mock_image = mock.Mock() mock_image.url = mock_logo_url @@ -895,7 +915,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.get_organization_by_short_name') def test_organization_missing(self, mock_get_organization_by_short_name): - """ Verify the logo image is not set if the organizations api returns None """ + """ + Verify the logo image is not set if the organizations api returns None. + """ mock_get_organization_by_short_name.return_value = None data = utils.ProgramDataExtender(self.program, self.user).extend() self.assertEqual(data['organizations'][0].get('img'), None)