Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #15636 from edx/clintonb/multi-tenant-programs-fix

Browse Source 
Resolved multi-tenant program issues
This commit is contained in:
Afzal Wali Naushahi
2017-07-25 20:28:35 +05:00
committed by GitHub
parent acad210d47 9efa7770f8
commit 0d7305bc49
12 changed files with 85 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/djangoapps/student/views.py
View File

@@ -739,7 +739,7 @@ def dashboard(request):
# Find programs associated with course runs being displayed. This information
# is passed in the template context to allow rendering of program-related
# information on the dashboard.
meter = ProgramProgressMeter(user, enrollments=course_enrollments)
meter = ProgramProgressMeter(request.site, user, enrollments=course_enrollments)
inverted_programs = meter.invert_programs()
# Construct a dictionary of course mode information

2
lms/djangoapps/courseware/views/views.py
View File

@@ -838,7 +838,7 @@ def program_marketing(request, program_uuid):
"""
Display the program marketing page.
"""
program_data = get_programs(uuid=program_uuid)
program_data = get_programs(request.site, uuid=program_uuid)
if not program_data:
raise Http404

4
lms/djangoapps/learner_dashboard/views.py
View File

@@ -25,7 +25,7 @@ def program_listing(request):
if not programs_config.enabled:
raise Http404
meter = ProgramProgressMeter(request.user)
meter = ProgramProgressMeter(request.site, request.user)
context = {
'disable_courseware_js': True,
@@ -48,7 +48,7 @@ def program_details(request, program_uuid):
if not programs_config.enabled:
raise Http404
meter = ProgramProgressMeter(request.user, uuid=program_uuid)
meter = ProgramProgressMeter(request.site, request.user, uuid=program_uuid)
program_data = meter.programs[0]
if not program_data:

27
openedx/core/djangoapps/catalog/tests/test_utils.py
View File

@@ -1,13 +1,13 @@
"""Tests covering utilities for integrating with the catalog service."""
# pylint: disable=missing-docstring
import copy
import uuid
import ddt
import mock
from django.contrib.auth import get_user_model
from django.core.cache import cache
from django.test import TestCase, override_settings
from student.tests.factories import UserFactory
from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, PROGRAM_UUIDS_CACHE_KEY
from openedx.core.djangoapps.catalog.models import CatalogIntegration
@@ -19,8 +19,8 @@ from openedx.core.djangoapps.catalog.utils import (
get_programs,
get_programs_with_type
)
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from student.tests.factories import UserFactory
UTILS_MODULE = 'openedx.core.djangoapps.catalog.utils'
User = get_user_model() # pylint: disable=invalid-name
@@ -32,6 +32,10 @@ User = get_user_model() # pylint: disable=invalid-name
class TestGetPrograms(CacheIsolationTestCase):
ENABLED_CACHES = ['default']
def setUp(self):
super(TestGetPrograms, self).setUp()
self.site = SiteFactory()
def test_get_many(self, mock_warning, mock_info):
programs = ProgramFactory.create_batch(3)
@@ -43,7 +47,7 @@ class TestGetPrograms(CacheIsolationTestCase):
# When called before UUIDs are cached, the function should return an
# empty list and log a warning.
self.assertEqual(get_programs(), [])
self.assertEqual(get_programs(self.site), [])
mock_warning.assert_called_once_with('Failed to get program UUIDs from the cache.')
mock_warning.reset_mock()
@@ -54,7 +58,7 @@ class TestGetPrograms(CacheIsolationTestCase):
None
)
actual_programs = get_programs()
actual_programs = get_programs(self.site)
# The 2 cached programs should be returned while info and warning
# messages should be logged for the missing one.
@@ -82,7 +86,7 @@ class TestGetPrograms(CacheIsolationTestCase):
}
cache.set_many(all_programs, None)
actual_programs = get_programs()
actual_programs = get_programs(self.site)
# All 3 programs should be returned.
self.assertEqual(
@@ -116,7 +120,7 @@ class TestGetPrograms(CacheIsolationTestCase):
mock_cache.get.return_value = [program['uuid'] for program in programs]
mock_cache.get_many.side_effect = fake_get_many
actual_programs = get_programs()
actual_programs = get_programs(self.site)
# All 3 cached programs should be returned. An info message should be
# logged about the one that was initially missing, but the code should
@@ -136,7 +140,7 @@ class TestGetPrograms(CacheIsolationTestCase):
expected_program = ProgramFactory()
expected_uuid = expected_program['uuid']
self.assertEqual(get_programs(uuid=expected_uuid), None)
self.assertEqual(get_programs(self.site, uuid=expected_uuid), None)
mock_warning.assert_called_once_with(
'Failed to get details for program {uuid} from the cache.'.format(uuid=expected_uuid)
)
@@ -148,7 +152,7 @@ class TestGetPrograms(CacheIsolationTestCase):
None
)
actual_program = get_programs(uuid=expected_uuid)
actual_program = get_programs(self.site, uuid=expected_uuid)
self.assertEqual(actual_program, expected_program)
self.assertFalse(mock_warning.called)
@@ -156,6 +160,9 @@ class TestGetPrograms(CacheIsolationTestCase):
@skip_unless_lms
@ddt.ddt
class TestGetProgramsWithType(TestCase):
def setUp(self):
super(TestGetProgramsWithType, self).setUp()
self.site = SiteFactory()
@mock.patch(UTILS_MODULE + '.get_programs')
@mock.patch(UTILS_MODULE + '.get_program_types')
@@ -176,7 +183,7 @@ class TestGetProgramsWithType(TestCase):
mock_get_programs.return_value = programs
mock_get_program_types.return_value = program_types
actual = get_programs_with_type()
actual = get_programs_with_type(self.site)
self.assertEqual(actual, programs_with_program_type)
@ddt.data(False, True)
@@ -202,7 +209,7 @@ class TestGetProgramsWithType(TestCase):
mock_get_programs.return_value = programs
mock_get_program_types.return_value = program_types
actual = get_programs_with_type(include_hidden=include_hidden)
actual = get_programs_with_type(self.site, include_hidden=include_hidden)
self.assertEqual(actual, programs_with_program_type)

15
openedx/core/djangoapps/catalog/utils.py
View File

@@ -14,7 +14,6 @@ from openedx.core.djangoapps.catalog.cache import (
SITE_PROGRAM_UUIDS_CACHE_KEY_TPL
)
from openedx.core.djangoapps.catalog.models import CatalogIntegration
from openedx.core.djangoapps.theming.helpers import get_current_site
from openedx.core.lib.edx_api_utils import get_edx_api_data
from openedx.core.lib.token_utils import JwtBuilder
@@ -35,11 +34,14 @@ def create_catalog_api_client(user, site=None):
return EdxRestApiClient(url, jwt=jwt)
def get_programs(uuid=None):
def get_programs(site, uuid=None):
"""Read programs from the cache.
The cache is populated by a management command, cache_programs.
Arguments:
site (Site): django.contrib.sites.models object
Keyword Arguments:
uuid (string): UUID identifying a specific program to read from the cache.
@@ -56,7 +58,7 @@ def get_programs(uuid=None):
return program
if waffle.switch_is_active('get-multitenant-programs'):
uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=get_current_site().domain), [])
uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), [])
else:
uuids = cache.get(PROGRAM_UUIDS_CACHE_KEY, [])
if not uuids:
@@ -121,13 +123,16 @@ def get_program_types(name=None):
return []
def get_programs_with_type(include_hidden=True):
def get_programs_with_type(site, include_hidden=True):
"""
Return the list of programs. You can filter the types of programs returned by using the optional
include_hidden parameter. By default hidden programs will be included.
The program dict is updated with the fully serialized program type.
Arguments:
site (Site): django.contrib.sites.models object
Keyword Arguments:
include_hidden (bool): whether to include hidden programs
@@ -135,7 +140,7 @@ def get_programs_with_type(include_hidden=True):
list of dict, representing the active programs.
"""
programs_with_type = []
programs = get_programs()
programs = get_programs(site)
if programs:
program_types = {program_type['name']: program_type for program_type in get_program_types()}

13
openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py
View File

@@ -1,18 +1,17 @@
"""Management command for backpopulating missing program credentials."""
from collections import namedtuple
import logging
from collections import namedtuple
from django.contrib.auth.models import User
from django.contrib.sites.models import Site
from django.core.management import BaseCommand
from django.db.models import Q
from opaque_keys.edx.keys import CourseKey
from certificates.models import GeneratedCertificate, CertificateStatuses # pylint: disable=import-error
from certificates.models import CertificateStatuses, GeneratedCertificate # pylint: disable=import-error
from course_modes.models import CourseMode
from openedx.core.djangoapps.catalog.utils import get_programs
from openedx.core.djangoapps.programs.tasks.v1.tasks import award_program_certificates
# TODO: Log to console, even with debug mode disabled?
logger = logging.getLogger(__name__) # pylint: disable=invalid-name
CourseRun = namedtuple('CourseRun', ['key', 'type'])
@@ -73,7 +72,11 @@ class Command(BaseCommand):
def _load_course_runs(self):
"""Find all course runs which are part of a program."""
programs = get_programs()
programs = []
for site in Site.objects.all():
logger.info('Loading programs from the catalog for site %s.', site.domain)
programs.extend(get_programs(site))
self.course_runs = self._flatten(programs)
def _flatten(self, programs):

19
openedx/core/djangoapps/programs/tasks/v1/tasks.py
View File

@@ -5,6 +5,7 @@ from celery import task
from celery.utils.log import get_task_logger # pylint: disable=no-name-in-module, import-error
from django.conf import settings
from django.contrib.auth.models import User
from django.contrib.sites.models import Site
from django.core.exceptions import ImproperlyConfigured
from edx_rest_api_client import exceptions
from edx_rest_api_client.client import EdxRestApiClient
@@ -55,18 +56,19 @@ def get_api_client(api_config, student):
return EdxRestApiClient(api_config.internal_api_url, jwt=jwt)
def get_completed_programs(student):
def get_completed_programs(site, student):
"""
Given a set of completed courses, determine which programs are completed.
Args:
site (Site): Site for which data should be retrieved.
student (User): Representing the student whose completed programs to check for.
Returns:
list of program UUIDs
"""
meter = ProgramProgressMeter(student)
meter = ProgramProgressMeter(site, student)
return meter.completed_programs
@@ -80,7 +82,7 @@ def get_certified_programs(student):
User object representing the student
Returns:
UUIDs of the programs for which the student has been awarded a certificate
str[]: UUIDs of the programs for which the student has been awarded a certificate
"""
certified_programs = []
@@ -129,8 +131,7 @@ def award_program_certificates(self, username):
student.
Args:
username:
The username of the student
username (str): The username of the student
Returns:
None
@@ -158,16 +159,16 @@ def award_program_certificates(self, username):
LOGGER.exception('Task award_program_certificates was called with invalid username %s', username)
# Don't retry for this case - just conclude the task.
return
program_uuids = get_completed_programs(student)
program_uuids = []
for site in Site.objects.all():
program_uuids.extend(get_completed_programs(site, 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)
return
# Determine which program certificates the user has already been
# awarded, if any.
# Determine which program certificates the user has already been awarded, if any.
existing_program_uuids = get_certified_programs(student)
except Exception as exc: # pylint: disable=broad-except

6
openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py
View File

@@ -16,6 +16,7 @@ from edx_rest_api_client.client import EdxRestApiClient
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
from openedx.core.djangoapps.programs.tasks.v1 import tasks
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
from student.tests.factories import UserFactory
@@ -130,6 +131,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
super(AwardProgramCertificatesTestCase, self).setUp()
self.create_credentials_config()
self.student = UserFactory.create(username='test-student')
self.site = SiteFactory()
self.catalog_integration = self.create_catalog_integration()
ClientFactory.create(name='credentials')
@@ -146,7 +148,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
programs.
"""
tasks.award_program_certificates.delay(self.student.username).get()
mock_get_completed_programs.assert_called_once_with(self.student)
mock_get_completed_programs.assert_called(self.site, self.student)
@ddt.data(
([1], [2, 3]),
@@ -282,7 +284,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
"""
mock_get_completed_programs.side_effect = self._make_side_effect([Exception('boom'), None])
tasks.award_program_certificates.delay(self.student.username).get()
self.assertEqual(mock_get_completed_programs.call_count, 2)
self.assertEqual(mock_get_completed_programs.call_count, 3)
def test_retry_on_credentials_api_errors(
self,

4
openedx/core/djangoapps/programs/tests/test_signals.py
View File

@@ -10,8 +10,8 @@ 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
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms, get_mock_request
TEST_USERNAME = 'test-user'

46
openedx/core/djangoapps/programs/tests/test_utils.py
View File

@@ -33,6 +33,7 @@ from openedx.core.djangoapps.programs.utils import (
ProgramMarketingDataExtender,
get_certificates,
)
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
from student.tests.factories import AnonymousUserFactory, UserFactory, CourseEnrollmentFactory
from util.date_utils import strftime_localized
@@ -55,6 +56,7 @@ class TestProgramProgressMeter(TestCase):
super(TestProgramProgressMeter, self).setUp()
self.user = UserFactory()
self.site = SiteFactory()
def _create_enrollments(self, *course_run_ids):
"""Variadic helper used to create course run enrollments."""
@@ -92,7 +94,7 @@ class TestProgramProgressMeter(TestCase):
data = [ProgramFactory()]
mock_get_programs.return_value = data
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self.assertEqual(meter.engaged_programs, [])
self._assert_progress(meter)
@@ -104,7 +106,7 @@ class TestProgramProgressMeter(TestCase):
course_run_id = generate_course_run_key()
self._create_enrollments(course_run_id)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self.assertEqual(meter.engaged_programs, [])
self._assert_progress(meter)
@@ -129,7 +131,7 @@ class TestProgramProgressMeter(TestCase):
mock_get_programs.return_value = data
self._create_enrollments(course_run_key)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._attach_detail_url(data)
program = data[0]
@@ -159,7 +161,7 @@ class TestProgramProgressMeter(TestCase):
self._create_enrollments(course_run_key)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
program = data[0]
expected = [
@@ -195,7 +197,7 @@ class TestProgramProgressMeter(TestCase):
mode=CourseMode.NO_ID_PROFESSIONAL_MODE
)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
program = data[0]
expected = [
@@ -236,7 +238,7 @@ class TestProgramProgressMeter(TestCase):
CourseEnrollmentFactory(user=self.user, course_id=course_run_key, mode='audit')
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
program = data[0]
expected = [
@@ -278,7 +280,7 @@ class TestProgramProgressMeter(TestCase):
# The creation time of the enrollments matters to the test. We want
# the first_course_run_key to represent the newest enrollment.
self._create_enrollments(older_course_run_key, newer_course_run_key)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._attach_detail_url(data)
programs = data[:2]
@@ -323,7 +325,7 @@ class TestProgramProgressMeter(TestCase):
# Enrollment for the shared course run created last (most recently).
self._create_enrollments(solo_course_run_key, shared_course_run_key)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._attach_detail_url(data)
programs = data[:3]
@@ -354,13 +356,13 @@ class TestProgramProgressMeter(TestCase):
mock_get_programs.return_value = data
# No enrollments, no programs in progress.
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._assert_progress(meter)
self.assertEqual(meter.completed_programs, [])
# One enrollment, one program in progress.
self._create_enrollments(first_course_run_key)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
program, program_uuid = data[0], data[0]['uuid']
self._assert_progress(
meter,
@@ -370,7 +372,7 @@ class TestProgramProgressMeter(TestCase):
# Two enrollments, all courses in progress.
self._create_enrollments(second_course_run_key)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._assert_progress(
meter,
ProgressFactory(uuid=program_uuid, in_progress=2)
@@ -381,7 +383,7 @@ class TestProgramProgressMeter(TestCase):
mock_completed_course_runs.return_value = [
{'course_run_id': first_course_run_key, 'type': MODES.verified},
]
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._assert_progress(
meter,
ProgressFactory(uuid=program_uuid, completed=1, in_progress=1)
@@ -393,7 +395,7 @@ class TestProgramProgressMeter(TestCase):
{'course_run_id': first_course_run_key, 'type': MODES.verified},
{'course_run_id': second_course_run_key, 'type': MODES.honor},
]
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._assert_progress(
meter,
ProgressFactory(uuid=program_uuid, completed=1, in_progress=1)
@@ -405,7 +407,7 @@ class TestProgramProgressMeter(TestCase):
{'course_run_id': first_course_run_key, 'type': MODES.verified},
{'course_run_id': second_course_run_key, 'type': MODES.verified},
]
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self._assert_progress(
meter,
ProgressFactory(uuid=program_uuid, completed=2)
@@ -436,7 +438,7 @@ class TestProgramProgressMeter(TestCase):
mock_completed_course_runs.return_value = [
{'course_run_id': course_run_key, 'type': MODES.honor},
]
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
program, program_uuid = data[0], data[0]['uuid']
self._assert_progress(
@@ -449,7 +451,7 @@ class TestProgramProgressMeter(TestCase):
"""Verify that programs with no courses do not count as completed."""
program = ProgramFactory()
program['courses'] = []
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
program_complete = meter._is_program_complete(program)
self.assertFalse(program_complete)
@@ -469,7 +471,7 @@ class TestProgramProgressMeter(TestCase):
course_run_keys.append(course_run['key'])
# Verify that no programs are complete.
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self.assertEqual(meter.completed_programs, [])
# Complete all programs.
@@ -480,7 +482,7 @@ class TestProgramProgressMeter(TestCase):
]
# Verify that all programs are complete.
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self.assertEqual(meter.completed_programs, program_uuids)
@mock.patch(UTILS_MODULE + '.certificate_api.get_certificates_for_user')
@@ -494,7 +496,7 @@ class TestProgramProgressMeter(TestCase):
self._make_certificate_result(status='unknown', course_key='unknown-course'),
]
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self.assertEqual(
meter.completed_course_runs,
[
@@ -517,7 +519,7 @@ class TestProgramProgressMeter(TestCase):
mock_get_programs.return_value = [program]
# Verify that the test program is not complete.
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self.assertEqual(meter.completed_programs, [])
# Grant a 'no-id-professional' certificate for one of the course runs,
@@ -527,7 +529,7 @@ class TestProgramProgressMeter(TestCase):
]
# Verify that the program is complete.
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
self.assertEqual(meter.completed_programs, [program['uuid']])
@mock.patch(UTILS_MODULE + '.ProgramProgressMeter.completed_course_runs', new_callable=mock.PropertyMock)
@@ -543,7 +545,7 @@ class TestProgramProgressMeter(TestCase):
program = ProgramFactory(courses=[course])
mock_get_programs.return_value = [program]
self._create_enrollments(course_run_key)
meter = ProgramProgressMeter(self.user)
meter = ProgramProgressMeter(self.site, self.user)
mock_completed_course_runs.return_value = [{'course_run_id': course_run_key, 'type': 'verified'}]
self.assertEqual(meter._is_course_complete(course), True)

7
openedx/core/djangoapps/programs/utils.py
View File

@@ -70,7 +70,8 @@ class ProgramProgressMeter(object):
will only inspect this one program, not all programs the user may be
engaged with.
"""
def __init__(self, user, enrollments=None, uuid=None):
def __init__(self, site, user, enrollments=None, uuid=None):
self.site = site
self.user = user
self.enrollments = enrollments or list(CourseEnrollment.enrollments_for_user(self.user))
@@ -89,9 +90,9 @@ class ProgramProgressMeter(object):
self.course_run_ids.append(enrollment_id)
if uuid:
self.programs = [get_programs(uuid=uuid)]
self.programs = [get_programs(self.site, uuid=uuid)]
else:
self.programs = attach_program_detail_url(get_programs())
self.programs = attach_program_detail_url(get_programs(self.site))
def invert_programs(self):
"""Intersect programs and enrollments.

2
openedx/core/djangoapps/site_configuration/tests/factories.py
View File

@@ -26,5 +26,7 @@ class SiteFactory(DjangoModelFactory):
model = Site
django_get_or_create = ('domain',)
# TODO These should be generated. Otherwise, code that creates multiple Site
# objects will only end up with a single Site since domain has a unique constraint.
domain = 'testserver.fake'
name = 'testserver.fake'