From 05b461824e71590c000be0f37d31163e9e4ca76c Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Wed, 4 Jan 2017 12:56:17 -0500 Subject: [PATCH] Pull programs from catalog when issuing program credentials Lays the groundwork for pulling all program data from the catalog. ECOM-6535 --- lms/djangoapps/support/tests/test_programs.py | 11 +- lms/djangoapps/support/views/programs.py | 7 - .../core/djangoapps/catalog/tests/mixins.py | 1 + openedx/core/djangoapps/catalog/utils.py | 2 +- openedx/core/djangoapps/credentials/models.py | 6 +- .../djangoapps/credentials/tests/factories.py | 4 +- .../djangoapps/credentials/tests/mixins.py | 13 +- .../credentials/tests/test_models.py | 4 +- .../credentials/tests/test_utils.py | 182 +++++++++++------- openedx/core/djangoapps/credentials/utils.py | 33 +++- .../backpopulate_program_credentials.py | 30 ++- openedx/core/djangoapps/programs/signals.py | 7 +- .../djangoapps/programs/tasks/v1/tasks.py | 73 ++++--- .../programs/tasks/v1/tests/test_tasks.py | 105 +++++----- .../test_backpopulate_program_credentials.py | 53 ++--- .../djangoapps/programs/tests/test_signals.py | 25 ++- .../djangoapps/programs/tests/test_utils.py | 68 ------- openedx/core/djangoapps/programs/utils.py | 64 ++---- 18 files changed, 298 insertions(+), 390 deletions(-) diff --git a/lms/djangoapps/support/tests/test_programs.py b/lms/djangoapps/support/tests/test_programs.py index 2bad288ddf..c75760e5f8 100644 --- a/lms/djangoapps/support/tests/test_programs.py +++ b/lms/djangoapps/support/tests/test_programs.py @@ -4,18 +4,15 @@ from django.test import TestCase import mock from edx_oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory -from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin from student.tests.factories import UserFactory -class IssueProgramCertificatesViewTests(TestCase, ProgramsApiConfigMixin): +class IssueProgramCertificatesViewTests(TestCase): password = 'password' def setUp(self): super(IssueProgramCertificatesViewTests, self).setUp() - self.create_programs_config() - self.path = reverse('support:programs-certify') self.user = UserFactory(password=self.password, is_staff=True) self.data = {'username': self.user.username} @@ -57,12 +54,6 @@ class IssueProgramCertificatesViewTests(TestCase, ProgramsApiConfigMixin): self._verify_response(403) - def test_certification_disabled(self): - """Verify that the endpoint returns a 400 when program certification is disabled.""" - self.create_programs_config(enable_certification=False) - - self._verify_response(400) - def test_username_required(self): """Verify that the endpoint returns a 400 when a username isn't provided.""" self.data.pop('username') diff --git a/lms/djangoapps/support/views/programs.py b/lms/djangoapps/support/views/programs.py index 2fdf72ae46..bd7aef710c 100644 --- a/lms/djangoapps/support/views/programs.py +++ b/lms/djangoapps/support/views/programs.py @@ -6,7 +6,6 @@ from rest_framework.authentication import SessionAuthentication from rest_framework.response import Response from rest_framework_oauth.authentication import OAuth2Authentication -from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.programs.tasks.v1.tasks import award_program_certificates @@ -38,12 +37,6 @@ class IssueProgramCertificatesView(views.APIView): permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser,) def post(self, request): - if not ProgramsApiConfig.current().is_certification_enabled: - return Response( - {'error': 'Program certification is disabled.'}, - status=status.HTTP_400_BAD_REQUEST - ) - username = request.data.get('username') if username: log.info('Enqueuing program certification task for user [%s]', username) diff --git a/openedx/core/djangoapps/catalog/tests/mixins.py b/openedx/core/djangoapps/catalog/tests/mixins.py index 2372a0b33f..db6142aadb 100644 --- a/openedx/core/djangoapps/catalog/tests/mixins.py +++ b/openedx/core/djangoapps/catalog/tests/mixins.py @@ -9,6 +9,7 @@ class CatalogIntegrationMixin(object): 'enabled': True, 'internal_api_url': 'https://catalog-internal.example.com/api/v1/', 'cache_ttl': 0, + 'service_username': 'lms_catalog_service_user' } def create_catalog_integration(self, **kwargs): diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index 64350d61a9..02684964ba 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -151,7 +151,7 @@ def munge_catalog_program(catalog_program): # The Programs schema only supports one organization here. 'display_name': course['owners'][0]['name'], 'key': course['owners'][0]['key'] - }, + } if course['owners'] else {}, 'run_modes': [ { 'course_key': run['key'], diff --git a/openedx/core/djangoapps/credentials/models.py b/openedx/core/djangoapps/credentials/models.py index 1f8f00a385..25fa2ce931 100644 --- a/openedx/core/djangoapps/credentials/models.py +++ b/openedx/core/djangoapps/credentials/models.py @@ -9,6 +9,8 @@ from django.db import models from config_models.models import ConfigurationModel +API_VERSION = 'v2' + class CredentialsApiConfig(ConfigurationModel): """ @@ -55,14 +57,14 @@ class CredentialsApiConfig(ConfigurationModel): """ Generate a URL based on internal service URL and API version number. """ - return urljoin(self.internal_service_url, '/api/v1/') + return urljoin(self.internal_service_url, '/api/{}/'.format(API_VERSION)) @property def public_api_url(self): """ Generate a URL based on public service URL and API version number. """ - return urljoin(self.public_service_url, '/api/v1/') + return urljoin(self.public_service_url, '/api/{}/'.format(API_VERSION)) @property def is_learner_issuance_enabled(self): diff --git a/openedx/core/djangoapps/credentials/tests/factories.py b/openedx/core/djangoapps/credentials/tests/factories.py index 62660ba76c..51ddf2324c 100644 --- a/openedx/core/djangoapps/credentials/tests/factories.py +++ b/openedx/core/djangoapps/credentials/tests/factories.py @@ -1,4 +1,6 @@ """Factories for generating fake credentials-related data.""" +import uuid + import factory from factory.fuzzy import FuzzyText @@ -26,7 +28,7 @@ class ProgramCredential(factory.Factory): model = dict credential_id = factory.Sequence(lambda n: n) - program_id = factory.Sequence(lambda n: n) + program_uuid = factory.LazyAttribute(lambda obj: str(uuid.uuid4())) class CourseCredential(factory.Factory): diff --git a/openedx/core/djangoapps/credentials/tests/mixins.py b/openedx/core/djangoapps/credentials/tests/mixins.py index 7561ec7f15..0ba9e7e9a8 100644 --- a/openedx/core/djangoapps/credentials/tests/mixins.py +++ b/openedx/core/djangoapps/credentials/tests/mixins.py @@ -37,16 +37,12 @@ class CredentialsDataMixin(object): factories.UserCredential( id=1, username='test', - credential=factories.ProgramCredential( - program_id=1 - ) + credential=factories.ProgramCredential() ), factories.UserCredential( id=2, username='test', - credential=factories.ProgramCredential( - program_id=2 - ) + credential=factories.ProgramCredential() ), factories.UserCredential( id=3, @@ -99,8 +95,7 @@ class CredentialsDataMixin(object): self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Credentials API calls.') internal_api_url = CredentialsApiConfig.current().internal_api_url.strip('/') - url = internal_api_url + '/user_credentials/?username=' + user.username - + url = internal_api_url + '/credentials/?status=awarded&username=' + user.username if reset_url: httpretty.reset() @@ -110,7 +105,7 @@ class CredentialsDataMixin(object): body = json.dumps(data) if is_next_page: - next_page_url = internal_api_url + '/user_credentials/?page=2&username=' + user.username + next_page_url = internal_api_url + '/credentials/?page=2&status=awarded&username=' + user.username self.CREDENTIALS_NEXT_API_RESPONSE['next'] = next_page_url next_page_body = json.dumps(self.CREDENTIALS_NEXT_API_RESPONSE) httpretty.register_uri( diff --git a/openedx/core/djangoapps/credentials/tests/test_models.py b/openedx/core/djangoapps/credentials/tests/test_models.py index 1e653610da..77178e15a7 100644 --- a/openedx/core/djangoapps/credentials/tests/test_models.py +++ b/openedx/core/djangoapps/credentials/tests/test_models.py @@ -17,11 +17,11 @@ class TestCredentialsApiConfig(CredentialsApiConfigMixin, TestCase): self.assertEqual( credentials_config.internal_api_url, - credentials_config.internal_service_url.strip('/') + '/api/v1/') + credentials_config.internal_service_url.strip('/') + '/api/v2/') self.assertEqual( credentials_config.public_api_url, - credentials_config.public_service_url.strip('/') + '/api/v1/') + credentials_config.public_service_url.strip('/') + '/api/v2/') def test_is_learner_issuance_enabled(self): """ diff --git a/openedx/core/djangoapps/credentials/tests/test_utils.py b/openedx/core/djangoapps/credentials/tests/test_utils.py index 443eec7029..d51a85f25a 100644 --- a/openedx/core/djangoapps/credentials/tests/test_utils.py +++ b/openedx/core/djangoapps/credentials/tests/test_utils.py @@ -1,16 +1,21 @@ """Tests covering Credentials utilities.""" +import uuid + from django.core.cache import cache -from nose.plugins.attrib import attr -import httpretty from edx_oauth2_provider.tests.factories import ClientFactory +import httpretty +import mock +from nose.plugins.attrib import attr from provider.constants import CONFIDENTIAL +from openedx.core.djangoapps.catalog.tests import factories as catalog_factories from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin, CredentialsDataMixin from openedx.core.djangoapps.credentials.utils import ( get_user_credentials, get_user_program_credentials, - get_programs_credentials + get_programs_credentials, + get_programs_for_credentials ) from openedx.core.djangoapps.credentials.tests import factories from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin, ProgramsDataMixin @@ -21,8 +26,7 @@ from student.tests.factories import UserFactory @skip_unless_lms @attr(shard=2) -class TestCredentialsRetrieval(ProgramsApiConfigMixin, CredentialsApiConfigMixin, CredentialsDataMixin, - ProgramsDataMixin, CacheIsolationTestCase): +class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, CacheIsolationTestCase): """ Tests covering the retrieval of user credentials from the Credentials service. """ @@ -35,41 +39,23 @@ class TestCredentialsRetrieval(ProgramsApiConfigMixin, CredentialsApiConfigMixin ClientFactory(name=CredentialsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) self.user = UserFactory() + self.primary_uuid = str(uuid.uuid4()) + self.alternate_uuid = str(uuid.uuid4()) cache.clear() - def _expected_progam_credentials_data(self): - """ - Dry method for getting expected program credentials response data. - """ - return [ - factories.UserCredential( - id=1, - username='test', - credential=factories.ProgramCredential(), - certificate_url=self.CREDENTIALS_API_RESPONSE['results'][0]['certificate_url'], - ), - factories.UserCredential( - id=2, - username='test', - credential=factories.ProgramCredential(), - certificate_url=self.CREDENTIALS_API_RESPONSE['results'][1]['certificate_url'], - ) - ] - - def expected_credentials_display_data(self): + def expected_credentials_display_data(self, programs): """ Returns expected credentials data to be represented. """ - program_credentials_data = self._expected_progam_credentials_data() return [ { - 'display_name': self.PROGRAMS_API_RESPONSE['results'][0]['name'], - 'subtitle': self.PROGRAMS_API_RESPONSE['results'][0]['subtitle'], - 'credential_url':program_credentials_data[0]['certificate_url'] + 'display_name': programs[0]['title'], + 'subtitle': programs[0]['subtitle'], + 'credential_url': programs[0]['credential_url'] }, { - 'display_name': self.PROGRAMS_API_RESPONSE['results'][1]['name'], - 'subtitle':self.PROGRAMS_API_RESPONSE['results'][1]['subtitle'], - 'credential_url':program_credentials_data[1]['certificate_url'] + 'display_name': programs[1]['title'], + 'subtitle': programs[1]['subtitle'], + 'credential_url': programs[1]['credential_url'] } ] @@ -125,41 +111,34 @@ class TestCredentialsRetrieval(ProgramsApiConfigMixin, CredentialsApiConfigMixin """Verify program credentials data can be retrieved and parsed correctly.""" # create credentials and program configuration self.create_credentials_config() - self.create_programs_config() # Mocking the API responses from programs and credentials - self.mock_programs_api() - self.mock_credentials_api(self.user, reset_url=False) + primary_uuid, alternate_uuid = str(uuid.uuid4()), str(uuid.uuid4()) + credentials_api_response = { + "next": None, + "results": [ + factories.UserCredential( + username='test', + credential=factories.ProgramCredential(program_uuid=primary_uuid) + ), + factories.UserCredential( + username='test', + credential=factories.ProgramCredential(program_uuid=alternate_uuid) + ) + ] + } + self.mock_credentials_api(self.user, data=credentials_api_response, reset_url=False) + programs = [ + catalog_factories.Program(uuid=primary_uuid), catalog_factories.Program(uuid=alternate_uuid) + ] - actual = get_user_program_credentials(self.user) - program_credentials_data = self._expected_progam_credentials_data() - expected = self.PROGRAMS_API_RESPONSE['results'][:2] - expected[0]['credential_url'] = program_credentials_data[0]['certificate_url'] - expected[1]['credential_url'] = program_credentials_data[1]['certificate_url'] + with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs_for_credentials") as mock_get_programs: + mock_get_programs.return_value = programs + actual = get_user_program_credentials(self.user) - # checking response from API is as expected - self.assertEqual(len(actual), 2) - self.assertEqual(actual, expected) - - @httpretty.activate - def test_get_user_program_credentials_revoked(self): - """Verify behavior if credential revoked.""" - self.create_credentials_config() - credential_data = {"results": [ - { - "id": 1, - "username": "test", - "credential": { - "credential_id": 1, - "program_id": 1 - }, - "status": "revoked", - "uuid": "dummy-uuid-1" - } - ]} - self.mock_credentials_api(self.user, data=credential_data) - actual = get_user_program_credentials(self.user) - self.assertEqual(actual, []) + # checking response from API is as expected + self.assertEqual(len(actual), 2) + self.assertEqual(actual, programs) @httpretty.activate def test_get_programs_credentials(self): @@ -168,14 +147,75 @@ class TestCredentialsRetrieval(ProgramsApiConfigMixin, CredentialsApiConfigMixin """ # create credentials and program configuration self.create_credentials_config() - self.create_programs_config() # Mocking the API responses from programs and credentials - self.mock_programs_api() - self.mock_credentials_api(self.user, reset_url=False) - actual = get_programs_credentials(self.user) - expected = self.expected_credentials_display_data() + primary_uuid, alternate_uuid = str(uuid.uuid4()), str(uuid.uuid4()) + credentials_api_response = { + "next": None, + "results": [ + factories.UserCredential( + username='test', + credential=factories.ProgramCredential(program_uuid=primary_uuid) + ), + factories.UserCredential( + username='test', + credential=factories.ProgramCredential(program_uuid=alternate_uuid) + ) + ] + } + self.mock_credentials_api(self.user, data=credentials_api_response, reset_url=False) + programs = [ + catalog_factories.Program(uuid=primary_uuid), catalog_factories.Program(uuid=alternate_uuid) + ] - # Checking result is as expected - self.assertEqual(len(actual), 2) - self.assertEqual(actual, expected) + with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs") as mock_get_programs: + mock_get_programs.return_value = programs + actual = get_programs_credentials(self.user) + expected = self.expected_credentials_display_data(programs) + + # Checking result is as expected + self.assertEqual(len(actual), 2) + self.assertEqual(actual, expected) + + def _expected_program_credentials_data(self): + """ + Dry method for getting expected program credentials response data. + """ + return [ + factories.UserCredential( + username='test', + credential=factories.ProgramCredential( + program_uuid=self.primary_uuid + ) + ), + factories.UserCredential( + username='test', + credential=factories.ProgramCredential( + program_uuid=self.alternate_uuid + ) + ) + ] + + def test_get_program_for_certificates(self): + """Verify programs data can be retrieved and parsed correctly for certificates.""" + programs = [ + catalog_factories.Program(uuid=self.primary_uuid), + catalog_factories.Program(uuid=self.alternate_uuid) + ] + + program_credentials_data = self._expected_program_credentials_data() + with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs") as patched_get_programs: + patched_get_programs.return_value = programs + actual = get_programs_for_credentials(self.user, program_credentials_data) + + self.assertEqual(len(actual), 2) + self.assertEqual(actual, programs) + + def test_get_program_for_certificates_no_data(self): + """Verify behavior when no programs data is found for the user.""" + program_credentials_data = self._expected_program_credentials_data() + with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs") as patched_get_programs: + patched_get_programs.return_value = [] + actual = get_programs_for_credentials(self.user, program_credentials_data) + + self.assertEqual(actual, []) diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index fc5df22d8d..e9aacd3df1 100644 --- a/openedx/core/djangoapps/credentials/utils.py +++ b/openedx/core/djangoapps/credentials/utils.py @@ -2,8 +2,8 @@ from __future__ import unicode_literals import logging +from openedx.core.djangoapps.catalog.utils import get_programs from openedx.core.djangoapps.credentials.models import CredentialsApiConfig -from openedx.core.djangoapps.programs.utils import get_programs_for_credentials from openedx.core.lib.edx_api_utils import get_edx_api_data @@ -19,18 +19,41 @@ def get_user_credentials(user): service. """ credential_configuration = CredentialsApiConfig.current() - user_query = {'username': user.username} + user_query = {'status': 'awarded', 'username': user.username} # Bypass caching for staff users, who may be generating credentials and # want to see them displayed immediately. use_cache = credential_configuration.is_cache_enabled and not user.is_staff cache_key = credential_configuration.CACHE_KEY + '.' + user.username if use_cache else None credentials = get_edx_api_data( - credential_configuration, user, 'user_credentials', querystring=user_query, cache_key=cache_key + credential_configuration, user, 'credentials', querystring=user_query, cache_key=cache_key ) return credentials +def get_programs_for_credentials(user, programs_credentials): + """ Given a user and an iterable of credentials, get corresponding programs + data and return it as a list of dictionaries. + + Arguments: + user (User): The user to authenticate as for requesting programs. + programs_credentials (list): List of credentials awarded to the user + for completion of a program. + + Returns: + list, containing programs dictionaries. + """ + certified_programs = [] + programs = get_programs(user) + for program in programs: + for credential in programs_credentials: + if program['uuid'] == credential['credential']['program_uuid']: + program['credential_url'] = credential['certificate_url'] + certified_programs.append(program) + + return certified_programs + + def get_user_program_credentials(user): """Given a user, get the list of all program credentials earned and returns list of dictionaries containing related programs data. @@ -55,7 +78,7 @@ def get_user_program_credentials(user): programs_credentials = [] for credential in credentials: try: - if 'program_id' in credential['credential'] and credential['status'] == 'awarded': + if 'program_uuid' in credential['credential']: programs_credentials.append(credential) except KeyError: log.exception('Invalid credential structure: %r', credential) @@ -84,7 +107,7 @@ def get_programs_credentials(user): for program in programs_credentials: try: program_data = { - 'display_name': program['name'], + 'display_name': program['title'], 'subtitle': program['subtitle'], 'credential_url': program['credential_url'], } diff --git a/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py b/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py index 2843433dba..3fad943317 100644 --- a/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py @@ -2,15 +2,16 @@ from collections import namedtuple import logging +from django.contrib.auth.models import User from django.core.management import BaseCommand, CommandError from django.db.models import Q from opaque_keys.edx.keys import CourseKey from provider.oauth2.models import Client from certificates.models import GeneratedCertificate, CertificateStatuses # pylint: disable=import-error -from openedx.core.djangoapps.programs.models import ProgramsApiConfig +from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.programs.tasks.v1.tasks import award_program_certificates -from openedx.core.djangoapps.programs.utils import get_programs +from openedx.core.djangoapps.catalog.utils import get_programs # TODO: Log to console, even with debug mode disabled? @@ -39,20 +40,17 @@ class Command(BaseCommand): ) def handle(self, *args, **options): - programs_config = ProgramsApiConfig.current() - self.client = Client.objects.get(name=programs_config.OAUTH2_CLIENT_NAME) + catalog_config = CatalogIntegration.current() - if self.client.user is None: - msg = ( - 'No user is associated with the {} OAuth2 client. ' - 'A service user is necessary to make requests to the Programs API. ' - 'No tasks have been enqueued. ' - 'Associate a user with the client and try again.' - ).format(programs_config.OAUTH2_CLIENT_NAME) + try: + user = User.objects.get(username=catalog_config.service_username) + except: + raise CommandError( + 'User with username [{}] not found. ' + 'A service user is required to run this command.'.format(catalog_config.service_username) + ) - raise CommandError(msg) - - self._load_run_modes() + self._load_run_modes(user) logger.info('Looking for users who may be eligible for a program certificate.') @@ -85,9 +83,9 @@ class Command(BaseCommand): failed ) - def _load_run_modes(self): + def _load_run_modes(self, user): """Find all run modes which are part of a program.""" - programs = get_programs(self.client.user) + programs = get_programs(user) self.run_modes = self._flatten(programs) def _flatten(self, programs): diff --git a/openedx/core/djangoapps/programs/signals.py b/openedx/core/djangoapps/programs/signals.py index 9991bbbb8a..56d5d85b17 100644 --- a/openedx/core/djangoapps/programs/signals.py +++ b/openedx/core/djangoapps/programs/signals.py @@ -35,10 +35,11 @@ def handle_course_cert_awarded(sender, user, course_key, mode, status, **kwargs) """ # Import here instead of top of file since this module gets imported before - # the programs app is loaded, resulting in a Django deprecation warning. - from openedx.core.djangoapps.programs.models import ProgramsApiConfig + # the credentials app is loaded, resulting in a Django deprecation warning. + from openedx.core.djangoapps.credentials.models import CredentialsApiConfig - if not ProgramsApiConfig.current().is_certification_enabled: + # Avoid scheduling new tasks if certification is disabled. + if not CredentialsApiConfig.current().is_learner_issuance_enabled: return # schedule background task to process diff --git a/openedx/core/djangoapps/programs/tasks/v1/tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tasks.py index 1b4fa4dea9..cce1cd7e88 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tasks.py @@ -11,7 +11,6 @@ from provider.oauth2.models import Client from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.utils import get_user_credentials -from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.programs.utils import ProgramProgressMeter from openedx.core.lib.token_utils import JwtBuilder @@ -19,6 +18,11 @@ from openedx.core.lib.token_utils import JwtBuilder LOGGER = get_task_logger(__name__) # Under cms the following setting is not defined, leading to errors during tests. ROUTING_KEY = getattr(settings, 'CREDENTIALS_GENERATION_ROUTING_KEY', None) +# Maximum number of retries before giving up on awarding credentials. +# For reference, 11 retries with exponential backoff yields a maximum waiting +# time of 2047 seconds (about 30 minutes). Setting this to None could yield +# unwanted behavior: infinite retries. +MAX_RETRIES = 11 def get_api_client(api_config, student): @@ -26,7 +30,7 @@ def get_api_client(api_config, student): Create and configure an API client for authenticated HTTP requests. Args: - api_config: ProgramsApiConfig or CredentialsApiConfig object + api_config: CredentialsApiConfig object student: User object as whom to authenticate to the API Returns: @@ -58,17 +62,16 @@ def get_completed_programs(student): student (User): Representing the student whose completed programs to check for. Returns: - list of program ids + list of program UUIDs """ - meter = ProgramProgressMeter(student) - + meter = ProgramProgressMeter(student, use_catalog=True) return meter.completed_programs -def get_awarded_certificate_programs(student): +def get_certified_programs(student): """ - Find the ids of all the programs for which the student has already been awarded + Find the UUIDs of all the programs for which the student has already been awarded a certificate. Args: @@ -76,17 +79,17 @@ def get_awarded_certificate_programs(student): User object representing the student Returns: - ids of the programs for which the student has been awarded a certificate + UUIDs of the programs for which the student has been awarded a certificate """ - return [ - credential['credential']['program_id'] - for credential in get_user_credentials(student) - if 'program_id' in credential['credential'] and credential['status'] == 'awarded' - ] + certified_programs = [] + for credential in get_user_credentials(student): + if 'program_uuid' in credential['credential']: + certified_programs.append(credential['credential']['program_uuid']) + return certified_programs -def award_program_certificate(client, username, program_id): +def award_program_certificate(client, username, program_uuid): """ Issue a new certificate of completion to the given student for the given program. @@ -95,16 +98,16 @@ def award_program_certificate(client, username, program_id): credentials API client (EdxRestApiClient) username: The username of the student - program_id: - id of the completed program + program_uuid: + uuid of the completed program Returns: None """ - client.user_credentials.post({ + client.credentials.post({ 'username': username, - 'credential': {'program_id': program_id}, + 'credential': {'program_uuid': program_uuid}, 'attributes': [] }) @@ -134,24 +137,18 @@ def award_program_certificates(self, username): """ LOGGER.info('Running task award_program_certificates for username %s', username) - config = ProgramsApiConfig.current() countdown = 2 ** self.request.retries - # If either programs or credentials config models are disabled for this + # If the credentials config model is disabled for this # feature, it may indicate a condition where processing of such tasks # has been temporarily disabled. Since this is a recoverable situation, # mark this task for retry instead of failing it altogether. - if not config.is_certification_enabled: - LOGGER.warning( - 'Task award_program_certificates cannot be executed when program certification is disabled in API config', - ) - raise self.retry(countdown=countdown, max_retries=config.max_retries) if not CredentialsApiConfig.current().is_learner_issuance_enabled: LOGGER.warning( 'Task award_program_certificates cannot be executed when credentials issuance is disabled in API config', ) - raise self.retry(countdown=countdown, max_retries=config.max_retries) + raise self.retry(countdown=countdown, max_retries=MAX_RETRIES) try: try: @@ -161,8 +158,8 @@ def award_program_certificates(self, username): # Don't retry for this case - just conclude the task. return - program_ids = get_completed_programs(student) - if not program_ids: + program_uuids = get_completed_programs(student) + if not program_uuids: # No reason to continue beyond this point unless/until this # task gets updated to support revocation of program certs. LOGGER.info('Task award_program_certificates was called for user %s with no completed programs', username) @@ -170,11 +167,11 @@ def award_program_certificates(self, username): # Determine which program certificates the user has already been # awarded, if any. - existing_program_ids = get_awarded_certificate_programs(student) + existing_program_uuids = get_certified_programs(student) except Exception as exc: # pylint: disable=broad-except LOGGER.exception('Failed to determine program certificates to be awarded for user %s', username) - raise self.retry(exc=exc, countdown=countdown, max_retries=config.max_retries) + raise self.retry(exc=exc, countdown=countdown, max_retries=MAX_RETRIES) # For each completed program for which the student doesn't already have a # certificate, award one now. @@ -182,8 +179,8 @@ def award_program_certificates(self, username): # This logic is important, because we will retry the whole task if awarding any particular program cert fails. # # N.B. the list is sorted to facilitate deterministic ordering, e.g. for tests. - new_program_ids = sorted(list(set(program_ids) - set(existing_program_ids))) - if new_program_ids: + new_program_uuids = sorted(list(set(program_uuids) - set(existing_program_uuids))) + if new_program_uuids: try: credentials_client = get_api_client( CredentialsApiConfig.current(), @@ -192,22 +189,22 @@ def award_program_certificates(self, username): except Exception as exc: # pylint: disable=broad-except LOGGER.exception('Failed to create a credentials API client to award program certificates') # Retry because a misconfiguration could be fixed - raise self.retry(exc=exc, countdown=countdown, max_retries=config.max_retries) + raise self.retry(exc=exc, countdown=countdown, max_retries=MAX_RETRIES) retry = False - for program_id in new_program_ids: + for program_uuid in new_program_uuids: try: - award_program_certificate(credentials_client, username, program_id) - LOGGER.info('Awarded certificate for program %s to user %s', program_id, username) + award_program_certificate(credentials_client, username, program_uuid) + LOGGER.info('Awarded certificate for program %s to user %s', program_uuid, username) except Exception: # pylint: disable=broad-except # keep trying to award other certs, but retry the whole task to fix any missing entries - LOGGER.exception('Failed to award certificate for program %s to user %s', program_id, username) + LOGGER.exception('Failed to award certificate for program %s to user %s', program_uuid, username) retry = True if retry: # N.B. This logic assumes that this task is idempotent LOGGER.info('Retrying task to award failed certificates to user %s', username) - raise self.retry(countdown=countdown, max_retries=config.max_retries) + raise self.retry(countdown=countdown, max_retries=MAX_RETRIES) else: LOGGER.info('User %s is not eligible for any new program certificates', username) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py index 73e360e88b..310a8e7289 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -15,20 +15,19 @@ import mock from provider.constants import CONFIDENTIAL from lms.djangoapps.certificates.api import MODES +from openedx.core.djangoapps.catalog.tests import factories, mixins +from openedx.core.djangoapps.catalog.utils import munge_catalog_program from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin -from openedx.core.djangoapps.programs.tests import factories -from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin from openedx.core.djangoapps.programs.tasks.v1 import tasks from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from student.tests.factories import UserFactory - TASKS_MODULE = 'openedx.core.djangoapps.programs.tasks.v1.tasks' UTILS_MODULE = 'openedx.core.djangoapps.programs.utils' @skip_unless_lms -class GetApiClientTestCase(TestCase, ProgramsApiConfigMixin): +class GetApiClientTestCase(CredentialsApiConfigMixin, TestCase): """ Test the get_api_client function """ @@ -39,21 +38,20 @@ class GetApiClientTestCase(TestCase, ProgramsApiConfigMixin): Ensure the function is making the right API calls based on inputs """ student = UserFactory() - ClientFactory.create(name='programs') - api_config = self.create_programs_config( - internal_service_url='http://foo', - api_version_number=99, + ClientFactory.create(name='credentials') + api_config = self.create_credentials_config( + internal_service_url='http://foo' ) mock_build_token.return_value = 'test-token' api_client = tasks.get_api_client(api_config, student) - self.assertEqual(api_client._store['base_url'], 'http://foo/api/v99/') # pylint: disable=protected-access + self.assertEqual(api_client._store['base_url'], 'http://foo/api/v2/') # pylint: disable=protected-access self.assertEqual(api_client._store['session'].auth.token, 'test-token') # pylint: disable=protected-access @httpretty.activate @skip_unless_lms -class GetCompletedProgramsTestCase(ProgramsApiConfigMixin, CacheIsolationTestCase): +class GetCompletedProgramsTestCase(mixins.CatalogIntegrationMixin, CacheIsolationTestCase): """ Test the get_completed_programs function """ @@ -63,19 +61,14 @@ class GetCompletedProgramsTestCase(ProgramsApiConfigMixin, CacheIsolationTestCas super(GetCompletedProgramsTestCase, self).setUp() self.user = UserFactory() - self.programs_config = self.create_programs_config(cache_ttl=5) - - ClientFactory(name=self.programs_config.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) - - cache.clear() + self.catalog_integration = self.create_catalog_integration(cache_ttl=1) def _mock_programs_api(self, data): """Helper for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') + self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock API calls.') - url = self.programs_config.internal_api_url.strip('/') + '/programs/' + url = self.catalog_integration.internal_api_url.strip('/') + '/programs/' body = json.dumps({'results': data}) - httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json') def _assert_num_requests(self, count): @@ -87,35 +80,30 @@ class GetCompletedProgramsTestCase(ProgramsApiConfigMixin, CacheIsolationTestCas """ Verify that completed programs are found, using the cache when possible. """ - course_id = 'org/course/run' data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=course_id), - ]), - ] - ), + factories.Program(), ] self._mock_programs_api(data) - mock_get_completed_courses.return_value = [ - {'course_id': course_id, 'mode': MODES.verified} - ] + munged_program = munge_catalog_program(data[0]) + course_codes = munged_program['course_codes'] + mock_get_completed_courses.return_value = [ + {'course_id': run_mode['course_key'], 'mode': run_mode['mode_slug']} + for run_mode in course_codes[0]['run_modes'] + ] for _ in range(2): result = tasks.get_completed_programs(self.user) - self.assertEqual(result, [data[0]['id']]) + self.assertEqual(result[0], munged_program['id']) - # Verify that only one request to programs was made (i.e., the cache was hit). + # Verify that only one request to the catalog was made (i.e., the cache was hit). self._assert_num_requests(1) @skip_unless_lms class GetAwardedCertificateProgramsTestCase(TestCase): """ - Test the get_awarded_certificate_programs function + Test the get_certified_programs function """ def make_credential_result(self, **kwargs): @@ -127,7 +115,7 @@ class GetAwardedCertificateProgramsTestCase(TestCase): 'username': 'dummy-username', 'credential': { 'credential_id': None, - 'program_id': None, + 'program_uuid': None, }, 'status': 'dummy-status', 'uuid': 'dummy-uuid', @@ -137,18 +125,17 @@ class GetAwardedCertificateProgramsTestCase(TestCase): return result @mock.patch(TASKS_MODULE + '.get_user_credentials') - def test_get_awarded_certificate_programs(self, mock_get_user_credentials): + def test_get_certified_programs(self, mock_get_user_credentials): """ Ensure the API is called and results handled correctly. """ student = UserFactory(username='test-username') mock_get_user_credentials.return_value = [ - self.make_credential_result(status='awarded', credential={'program_id': 1}), + self.make_credential_result(status='awarded', credential={'program_uuid': 1}), self.make_credential_result(status='awarded', credential={'course_id': 2}), - self.make_credential_result(status='revoked', credential={'program_id': 3}), ] - result = tasks.get_awarded_certificate_programs(student) + result = tasks.get_certified_programs(student) self.assertEqual(mock_get_user_credentials.call_args[0], (student, )) self.assertEqual(result, [1]) @@ -169,14 +156,14 @@ class AwardProgramCertificateTestCase(TestCase): httpretty.register_uri( httpretty.POST, - 'http://test-server/user_credentials/', + 'http://test-server/credentials/', ) tasks.award_program_certificate(test_client, test_username, 123) expected_body = { 'username': test_username, - 'credential': {'program_id': 123}, + 'credential': {'program_uuid': 123}, 'attributes': [] } self.assertEqual(json.loads(httpretty.last_request().body), expected_body) @@ -185,28 +172,27 @@ class AwardProgramCertificateTestCase(TestCase): @skip_unless_lms @ddt.ddt @mock.patch(TASKS_MODULE + '.award_program_certificate') -@mock.patch(TASKS_MODULE + '.get_awarded_certificate_programs') +@mock.patch(TASKS_MODULE + '.get_certified_programs') @mock.patch(TASKS_MODULE + '.get_completed_programs') @override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username') -class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, CredentialsApiConfigMixin): +class AwardProgramCertificatesTestCase(mixins.CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): """ Tests for the 'award_program_certificates' celery task. """ def setUp(self): super(AwardProgramCertificatesTestCase, self).setUp() - self.create_programs_config() self.create_credentials_config() self.student = UserFactory.create(username='test-student') - ClientFactory.create(name='programs') + self.catalog_integration = self.create_catalog_integration() ClientFactory.create(name='credentials') UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME) # pylint: disable=no-member def test_completion_check( self, mock_get_completed_programs, - mock_get_awarded_certificate_programs, # pylint: disable=unused-argument + mock_get_certified_programs, # pylint: disable=unused-argument mock_award_program_certificate, # pylint: disable=unused-argument ): """ @@ -224,10 +210,10 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent @ddt.unpack def test_awarding_certs( self, - already_awarded_program_ids, - expected_awarded_program_ids, + already_awarded_program_uuids, + expected_awarded_program_uuids, mock_get_completed_programs, - mock_get_awarded_certificate_programs, + mock_get_certified_programs, mock_award_program_certificate, ): """ @@ -235,15 +221,14 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent the proper programs. """ mock_get_completed_programs.return_value = [1, 2, 3] - mock_get_awarded_certificate_programs.return_value = already_awarded_program_ids + mock_get_certified_programs.return_value = already_awarded_program_uuids tasks.award_program_certificates.delay(self.student.username).get() - actual_program_ids = [call[0][2] for call in mock_award_program_certificate.call_args_list] - self.assertEqual(actual_program_ids, expected_awarded_program_ids) + actual_program_uuids = [call[0][2] for call in mock_award_program_certificate.call_args_list] + self.assertEqual(actual_program_uuids, expected_awarded_program_uuids) @ddt.data( - ('programs', 'enable_certification'), ('credentials', 'enable_learner_issuance'), ) @ddt.unpack @@ -279,7 +264,7 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent def test_abort_if_no_completed_programs( self, mock_get_completed_programs, - mock_get_awarded_certificate_programs, + mock_get_certified_programs, mock_award_program_certificate, ): """ @@ -289,7 +274,7 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent mock_get_completed_programs.return_value = [] tasks.award_program_certificates.delay(self.student.username).get() self.assertTrue(mock_get_completed_programs.called) - self.assertFalse(mock_get_awarded_certificate_programs.called) + self.assertFalse(mock_get_certified_programs.called) self.assertFalse(mock_award_program_certificate.called) def _make_side_effect(self, side_effects): @@ -314,7 +299,7 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent def test_continue_awarding_certs_if_error( self, mock_get_completed_programs, - mock_get_awarded_certificate_programs, + mock_get_certified_programs, mock_award_program_certificate, ): """ @@ -324,7 +309,7 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent that arise are logged also. """ mock_get_completed_programs.return_value = [1, 2] - mock_get_awarded_certificate_programs.side_effect = [[], [2]] + mock_get_certified_programs.side_effect = [[], [2]] mock_award_program_certificate.side_effect = self._make_side_effect([Exception('boom'), None]) with mock.patch(TASKS_MODULE + '.LOGGER.info') as mock_info, \ @@ -354,7 +339,7 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent def test_retry_on_credentials_api_errors( self, mock_get_completed_programs, - mock_get_awarded_certificate_programs, + mock_get_certified_programs, mock_award_program_certificate, ): """ @@ -364,8 +349,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent retry. """ mock_get_completed_programs.return_value = [1, 2] - mock_get_awarded_certificate_programs.return_value = [1] - mock_get_awarded_certificate_programs.side_effect = self._make_side_effect([Exception('boom'), None]) + mock_get_certified_programs.return_value = [1] + mock_get_certified_programs.side_effect = self._make_side_effect([Exception('boom'), None]) tasks.award_program_certificates.delay(self.student.username).get() - self.assertEqual(mock_get_awarded_certificate_programs.call_count, 2) + self.assertEqual(mock_get_certified_programs.call_count, 2) self.assertEqual(mock_award_program_certificate.call_count, 1) diff --git a/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py b/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py index 9382139210..42276fc98d 100644 --- a/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py @@ -4,17 +4,15 @@ import json import ddt from django.core.management import call_command, CommandError from django.test import TestCase -from edx_oauth2_provider.tests.factories import ClientFactory import httpretty import mock -from provider.constants import CONFIDENTIAL from certificates.models import CertificateStatuses # pylint: disable=import-error from lms.djangoapps.certificates.api import MODES from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory -from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.programs.tests import factories -from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin +from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory @@ -26,7 +24,7 @@ COMMAND_MODULE = 'openedx.core.djangoapps.programs.management.commands.backpopul @httpretty.activate @mock.patch(COMMAND_MODULE + '.award_program_certificates.delay') @skip_unless_lms -class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): +class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): """Tests for the backpopulate_program_credentials management command.""" course_id, alternate_course_id = 'org/course/run', 'org/alternate/run' @@ -35,24 +33,20 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): self.alice = UserFactory() self.bob = UserFactory() - self.oauth2_user = UserFactory() - self.oauth2_client = ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) # Disable certification to prevent the task from being triggered when # setting up test data (i.e., certificates with a passing status), thereby # skewing mock call counts. - self.create_programs_config(enable_certification=False) + self.create_credentials_config(enable_learner_issuance=False) - def _link_oauth2_user(self): - """Helper to link user and OAuth2 client.""" - self.oauth2_client.user = self.oauth2_user - self.oauth2_client.save() # pylint: disable=no-member + self.catalog_integration = self.create_catalog_integration() + self.service_user = UserFactory(username=self.catalog_integration.service_username) def _mock_programs_api(self, data): - """Helper for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') + """Helper for mocking out Catalog API URLs.""" + self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Catalog API calls.') - url = ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/' + url = self.catalog_integration.internal_api_url.strip('/') + '/programs/' body = json.dumps({'results': data}) httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json') @@ -71,7 +65,6 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): ), ] self._mock_programs_api(data) - self._link_oauth2_user() GeneratedCertificateFactory( user=self.alice, @@ -141,7 +134,6 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): def test_handle_flatten(self, data, mock_task): """Verify that program structures are flattened correctly.""" self._mock_programs_api(data) - self._link_oauth2_user() GeneratedCertificateFactory( user=self.alice, @@ -179,7 +171,6 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): ), ] self._mock_programs_api(data) - self._link_oauth2_user() GeneratedCertificateFactory( user=self.alice, @@ -215,7 +206,6 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): ), ] self._mock_programs_api(data) - self._link_oauth2_user() GeneratedCertificateFactory( user=self.alice, @@ -248,7 +238,6 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): ), ] self._mock_programs_api(data) - self._link_oauth2_user() passing_status = CertificateStatuses.downloadable failing_status = CertificateStatuses.notpassing @@ -276,27 +265,10 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): mock_task.assert_called_once_with(self.alice.username) - def test_handle_unlinked_oauth2_user(self, mock_task): - """Verify that the command fails when no user is associated with the OAuth2 client.""" - data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=self.course_id), - ]), - ] - ), - ] - self._mock_programs_api(data) - - GeneratedCertificateFactory( - user=self.alice, - course_id=self.course_id, - mode=MODES.verified, - status=CertificateStatuses.downloadable, - ) + def test_handle_missing_service_user(self, mock_task): + """Verify that the command fails when no service user exists.""" + self.catalog_integration = self.create_catalog_integration(service_username='test') with self.assertRaises(CommandError): call_command('backpopulate_program_credentials') @@ -323,7 +295,6 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): ), ] self._mock_programs_api(data) - self._link_oauth2_user() GeneratedCertificateFactory( user=self.alice, diff --git a/openedx/core/djangoapps/programs/tests/test_signals.py b/openedx/core/djangoapps/programs/tests/test_signals.py index d3540e9501..30950b27c5 100644 --- a/openedx/core/djangoapps/programs/tests/test_signals.py +++ b/openedx/core/djangoapps/programs/tests/test_signals.py @@ -10,15 +10,18 @@ from student.tests.factories import UserFactory from openedx.core.djangoapps.signals.signals import COURSE_CERT_AWARDED from openedx.core.djangoapps.programs.signals import handle_course_cert_awarded +from openedx.core.djangolib.testing.utils import skip_unless_lms TEST_USERNAME = 'test-user' @attr(shard=2) +# The credentials app isn't installed for the CMS. +@skip_unless_lms @mock.patch('openedx.core.djangoapps.programs.tasks.v1.tasks.award_program_certificates.delay') @mock.patch( - 'openedx.core.djangoapps.programs.models.ProgramsApiConfig.is_certification_enabled', + 'openedx.core.djangoapps.credentials.models.CredentialsApiConfig.is_learner_issuance_enabled', new_callable=mock.PropertyMock, return_value=False, ) @@ -40,7 +43,7 @@ class CertAwardedReceiverTest(TestCase): status='test-status', ) - def test_signal_received(self, mock_is_certification_enabled, mock_task): # pylint: disable=unused-argument + def test_signal_received(self, mock_is_learner_issuance_enabled, mock_task): # pylint: disable=unused-argument """ Ensures the receiver function is invoked when COURSE_CERT_AWARDED is sent. @@ -50,24 +53,26 @@ class CertAwardedReceiverTest(TestCase): known to take place inside the function. """ COURSE_CERT_AWARDED.send(**self.signal_kwargs) - self.assertEqual(mock_is_certification_enabled.call_count, 1) + self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1) - def test_programs_disabled(self, mock_is_certification_enabled, mock_task): + def test_programs_disabled(self, mock_is_learner_issuance_enabled, mock_task): """ - Ensures that the receiver function does nothing when the programs API + Ensures that the receiver function does nothing when the credentials API configuration is not enabled. """ handle_course_cert_awarded(**self.signal_kwargs) - self.assertEqual(mock_is_certification_enabled.call_count, 1) + self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1) self.assertEqual(mock_task.call_count, 0) - def test_programs_enabled(self, mock_is_certification_enabled, mock_task): + def test_programs_enabled(self, mock_is_learner_issuance_enabled, mock_task): """ Ensures that the receiver function invokes the expected celery task - when the programs API configuration is enabled. + when the credentials API configuration is enabled. """ - mock_is_certification_enabled.return_value = True + mock_is_learner_issuance_enabled.return_value = True + handle_course_cert_awarded(**self.signal_kwargs) - self.assertEqual(mock_is_certification_enabled.call_count, 1) + + self.assertEqual(mock_is_learner_issuance_enabled.call_count, 1) self.assertEqual(mock_task.call_count, 1) self.assertEqual(mock_task.call_args[0], (TEST_USERNAME,)) diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index f72bd4f5a7..91bf6aeb17 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -21,7 +21,6 @@ from pytz import utc from lms.djangoapps.certificates.api import MODES from lms.djangoapps.commerce.tests.test_utils import update_commerce_config 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 from openedx.core.djangoapps.programs import utils from openedx.core.djangoapps.programs.models import ProgramsApiConfig @@ -58,27 +57,6 @@ class TestProgramRetrieval(ProgramsApiConfigMixin, ProgramsDataMixin, Credential cache.clear() - def _expected_progam_credentials_data(self): - """ - Dry method for getting expected program credentials response data. - """ - return [ - credentials_factories.UserCredential( - id=1, - username='test', - credential=credentials_factories.ProgramCredential( - program_id=1 - ) - ), - credentials_factories.UserCredential( - id=2, - username='test', - credential=credentials_factories.ProgramCredential( - program_id=2 - ) - ) - ] - def test_get_programs(self): """Verify programs data can be retrieved.""" self.create_programs_config() @@ -141,52 +119,6 @@ class TestProgramRetrieval(ProgramsApiConfigMixin, ProgramsDataMixin, Credential actual = utils.get_programs(self.user) self.assertEqual(actual, []) - def test_get_program_for_certificates(self): - """Verify programs data can be retrieved and parsed correctly for certificates.""" - self.create_programs_config() - self.mock_programs_api() - program_credentials_data = self._expected_progam_credentials_data() - - actual = utils.get_programs_for_credentials(self.user, program_credentials_data) - expected = self.PROGRAMS_API_RESPONSE['results'][:2] - expected[0]['credential_url'] = program_credentials_data[0]['certificate_url'] - expected[1]['credential_url'] = program_credentials_data[1]['certificate_url'] - - self.assertEqual(len(actual), 2) - self.assertEqual(actual, expected) - - def test_get_program_for_certificates_no_data(self): - """Verify behavior when no programs data is found for the user.""" - self.create_programs_config() - self.create_credentials_config() - self.mock_programs_api(data={'results': []}) - program_credentials_data = self._expected_progam_credentials_data() - - actual = utils.get_programs_for_credentials(self.user, program_credentials_data) - self.assertEqual(actual, []) - - def test_get_program_for_certificates_id_not_exist(self): - """Verify behavior when no program with the given program_id in - credentials exists. - """ - self.create_programs_config() - self.create_credentials_config() - self.mock_programs_api() - credential_data = [ - { - "id": 1, - "username": "test", - "credential": { - "credential_id": 1, - "program_id": 100 - }, - "status": "awarded", - "credential_url": "www.example.com" - } - ] - actual = utils.get_programs_for_credentials(self.user, credential_data) - self.assertEqual(actual, []) - @skip_unless_lms class GetProgramsByRunTests(TestCase): diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index abe4e30f05..0a43e139d5 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- """Helper functions for working with Programs.""" import datetime -import logging from urlparse import urljoin from django.conf import settings @@ -26,13 +25,11 @@ from util.date_utils import strftime_localized from util.organizations_helpers import get_organization_by_short_name -log = logging.getLogger(__name__) - # The datetime module's strftime() methods require a year >= 1900. DEFAULT_ENROLLMENT_START_DATE = datetime.datetime(1900, 1, 1, tzinfo=utc) -def get_programs(user, program_id=None): +def get_programs(user, program_id=None, use_catalog=False): """Given a user, get programs from the Programs service. Returned value is cached depending on user permissions. Staff users making requests @@ -49,51 +46,27 @@ def get_programs(user, program_id=None): list of dict, representing programs returned by the Programs service. dict, if a specific program is requested. """ - programs_config = ProgramsApiConfig.current() - # Bypass caching for staff users, who may be creating Programs and want - # to see them displayed immediately. - cache_key = programs_config.CACHE_KEY if programs_config.is_cache_enabled and not user.is_staff else None + if use_catalog: + programs = [munge_catalog_program(program) for program in get_catalog_programs(user)] + else: + programs_config = ProgramsApiConfig.current() - programs = get_edx_api_data(programs_config, user, 'programs', resource_id=program_id, cache_key=cache_key) + # Bypass caching for staff users, who may be creating Programs and want + # to see them displayed immediately. + cache_key = programs_config.CACHE_KEY if programs_config.is_cache_enabled and not user.is_staff else None - # Mix in munged MicroMasters data from the catalog. - if not program_id: - programs += [ - munge_catalog_program(micromaster) for micromaster in get_catalog_programs(user, type='MicroMasters') - ] + programs = get_edx_api_data(programs_config, user, 'programs', resource_id=program_id, cache_key=cache_key) + + # Mix in munged MicroMasters data from the catalog. + if not program_id: + programs += [ + munge_catalog_program(micromaster) for micromaster in get_catalog_programs(user, type='MicroMasters') + ] return programs -def get_programs_for_credentials(user, programs_credentials): - """ Given a user and an iterable of credentials, get corresponding programs - data and return it as a list of dictionaries. - - Arguments: - user (User): The user to authenticate as for requesting programs. - programs_credentials (list): List of credentials awarded to the user - for completion of a program. - - Returns: - list, containing programs dictionaries. - """ - certificate_programs = [] - - programs = get_programs(user) - if not programs: - log.debug('No programs for user %d.', user.id) - return certificate_programs - - for program in programs: - for credential in programs_credentials: - if program['id'] == credential['credential']['program_id']: - program['credential_url'] = credential['certificate_url'] - certificate_programs.append(program) - - return certificate_programs - - def get_programs_by_run(programs, enrollments): """Intersect programs and enrollments. @@ -147,7 +120,6 @@ def attach_program_detail_url(programs): for program in programs: base = reverse('program_details_view', kwargs={'program_id': program['id']}).rstrip('/') slug = slugify(program['name']) - program['detail_url'] = '{base}/{slug}'.format(base=base, slug=slug) return programs @@ -182,13 +154,14 @@ class ProgramProgressMeter(object): Keyword Arguments: enrollments (list): List of the user's enrollments. """ - def __init__(self, user, enrollments=None): + def __init__(self, user, enrollments=None, use_catalog=False): self.user = user self.enrollments = enrollments self.course_ids = None self.course_certs = None + self.use_catalog = use_catalog - self.programs = attach_program_detail_url(get_programs(self.user)) + self.programs = attach_program_detail_url(get_programs(self.user, use_catalog=use_catalog)) def engaged_programs(self, by_run=False): """Derive a list of programs in which the given user is engaged. @@ -279,7 +252,6 @@ class ProgramProgressMeter(object): bool, whether the course code is complete. """ self.course_certs = self.course_certs or get_completed_courses(self.user) - return any(self._parse(run_mode) in self.course_certs for run_mode in course_code['run_modes']) def _is_course_code_in_progress(self, course_code):