EDUCATOR-4543 | Add utility functions to the catalog module that picks out all of the course_runs associated with a program, even those that live inside the curriculum of a sub-program. Use these functions to populate the course -> program index of the catalog/programs cache. Also some refactoring of a program_enrollments overview endpoint.

This commit is contained in:
Alex Dusenbery
2019-08-01 14:28:56 -04:00
committed by Alex Dusenbery
parent 98f740f71a
commit 70e2aaa95b
7 changed files with 297 additions and 102 deletions

View File

@@ -1327,8 +1327,8 @@ class ProgramCourseEnrollmentOverviewViewTests(ProgramCacheTestCaseMixin, Shared
# only freeze time when defining these values and not on the whole test case
# as test_multiple_enrollments_all_enrolled relies on actual differences in modified datetimes
with freeze_time('2019-01-01'):
cls.yesterday = datetime.now() - timedelta(1)
cls.tomorrow = datetime.now() + timedelta(1)
cls.yesterday = datetime.utcnow() - timedelta(1)
cls.tomorrow = datetime.utcnow() + timedelta(1)
cls.certificate_download_url = 'www.certificates.com'
@@ -1670,7 +1670,7 @@ class ProgramCourseEnrollmentOverviewViewTests(ProgramCacheTestCaseMixin, Shared
# course run has not ended and user has earned a passing certificate more than 30 days ago
certificate = self.create_generated_certificate()
certificate.created_date = datetime.now() - timedelta(30)
certificate.created_date = datetime.utcnow() - timedelta(30)
certificate.save()
mock_has_ended.return_value = False
@@ -1704,7 +1704,7 @@ class ProgramCourseEnrollmentOverviewViewTests(ProgramCacheTestCaseMixin, Shared
# course run has not ended and user has earned a passing certificate fewer than 30 days ago
certificate = self.create_generated_certificate()
certificate.created_date = datetime.now() - timedelta(5)
certificate.created_date = datetime.utcnow() - timedelta(5)
certificate.save()
response = self.client.get(self.get_url(self.program_uuid))

View File

@@ -12,6 +12,7 @@ from pytz import UTC
from django.http import Http404
from django.core.exceptions import PermissionDenied
from django.urls import reverse
from django.utils.functional import cached_property
from edx_rest_framework_extensions import permissions
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
@@ -567,7 +568,7 @@ class ProgramSpecificViewMixin(object):
A mixin for views that operate on or within a specific program.
"""
@property
@cached_property
def program(self):
"""
The program specified by the `program_uuid` URL parameter.
@@ -956,23 +957,77 @@ class ProgramCourseEnrollmentOverviewView(DeveloperErrorViewMixin, ProgramSpecif
for a user as part of a program.
"""
user = request.user
program_enrollment = self._get_program_enrollment(user, program_uuid)
user_program_enrollment = ProgramEnrollment.objects.filter(
program_course_enrollments = ProgramCourseEnrollment.objects.filter(
program_enrollment=program_enrollment
).select_related('course_enrollment')
enrollments_by_course_key = {
enrollment.course_key: enrollment.course_enrollment
for enrollment in program_course_enrollments
}
overviews = CourseOverview.get_from_ids_if_exists(enrollments_by_course_key.keys())
course_run_resume_urls = get_resume_urls_for_enrollments(user, enrollments_by_course_key.values())
course_runs = []
for enrollment in program_course_enrollments:
overview = overviews[enrollment.course_key]
certificate_info = get_certificate_for_user(user.username, enrollment.course_key) or {}
course_run_dict = {
'course_run_id': enrollment.course_key,
'display_name': overview.display_name_with_default,
'course_run_status': self.get_course_run_status(overview, certificate_info),
'course_run_url': self.get_course_run_url(request, enrollment.course_key),
'start_date': overview.start,
'end_date': overview.end,
'due_dates': self.get_due_dates(request, enrollment.course_key, user),
}
emails_enabled = self.get_emails_enabled(user, enrollment.course_key)
if emails_enabled is not None:
course_run_dict['emails_enabled'] = emails_enabled
if certificate_info.get('download_url'):
course_run_dict['certificate_download_url'] = certificate_info['download_url']
if self.program['type'] == 'MicroMasters':
course_run_dict['micromasters_title'] = self.program['title']
if course_run_resume_urls.get(enrollment.course_key):
course_run_dict['resume_course_run_url'] = course_run_resume_urls.get(enrollment.course_key)
course_runs.append(course_run_dict)
serializer = CourseRunOverviewListSerializer({'course_runs': course_runs})
return Response(serializer.data)
@staticmethod
def _get_program_enrollment(user, program_uuid):
"""
Returns the ``ProgramEnrollment`` record for the given program UUID in which the given
user is actively enrolled.
In the unusual and unlikely case of a user having two active program enrollments
for the same program, returns the most recently modified enrollment and logs
a warning.
Raises ``PermissionDenied`` if the user is not enrolled in the program with the given UUID.
"""
program_enrollment = ProgramEnrollment.objects.filter(
program_uuid=program_uuid,
user=user,
status='enrolled',
).order_by(
'-modified',
)
).order_by('-modified')
user_program_enrollment_count = user_program_enrollment.count()
program_enrollment_count = program_enrollment.count()
if user_program_enrollment_count > 1:
# in the unusual and unlikely case of a user having two
# active program enrollments for the same program,
# choose the most recently modified enrollment and log
# a warning
user_program_enrollment = user_program_enrollment[0]
if program_enrollment_count > 1:
program_enrollment = program_enrollment[0]
logger.warning(
('User with user_id {} has more than program enrollment'
'with an enrolled status for program uuid {}.').format(
@@ -980,68 +1035,10 @@ class ProgramCourseEnrollmentOverviewView(DeveloperErrorViewMixin, ProgramSpecif
program_uuid,
)
)
elif user_program_enrollment_count == 0:
# if the user is not enrolled in the program, they are not authorized
# to view the information returned by this endpoint
elif program_enrollment_count == 0:
raise PermissionDenied
user_program_course_enrollments = ProgramCourseEnrollment.objects.filter(
program_enrollment=user_program_enrollment
).select_related('course_enrollment')
enrollment_dict = {enrollment.course_key: enrollment.course_enrollment for enrollment in user_program_course_enrollments}
overviews = CourseOverview.get_from_ids_if_exists(enrollment_dict.keys())
resume_course_run_urls = get_resume_urls_for_enrollments(user, enrollment_dict.values())
response = {
'course_runs': [],
}
for enrollment in user_program_course_enrollments:
overview = overviews[enrollment.course_key]
certificate_download_url = None
is_certificate_passing = None
certificate_creation_date = None
certificate_info = get_certificate_for_user(user.username, enrollment.course_key)
if certificate_info:
certificate_download_url = certificate_info['download_url']
is_certificate_passing = certificate_info['is_passing']
certificate_creation_date = certificate_info['created']
course_run_dict = {
'course_run_id': enrollment.course_key,
'display_name': overview.display_name_with_default,
'course_run_status': self.get_course_run_status(overview, is_certificate_passing, certificate_creation_date),
'course_run_url': self.get_course_run_url(request, enrollment.course_key),
'start_date': overview.start,
'end_date': overview.end,
'due_dates': self.get_due_dates(request, enrollment.course_key, user),
}
if certificate_download_url:
course_run_dict['certificate_download_url'] = certificate_download_url
emails_enabled = self.get_emails_enabled(user, enrollment.course_key)
if emails_enabled is not None:
course_run_dict['emails_enabled'] = emails_enabled
micromasters_title = self.program['title'] if self.program['type'] == 'MicroMasters' else None
if micromasters_title:
course_run_dict['micromasters_title'] = micromasters_title
# if the url is '', then the url is None so we can omit it from the response
resume_course_run_url = resume_course_run_urls[enrollment.course_key]
if resume_course_run_url:
course_run_dict['resume_course_run_url'] = resume_course_run_url
response['course_runs'].append(course_run_dict)
serializer = CourseRunOverviewListSerializer(response)
return Response(serializer.data)
return program_enrollment
@staticmethod
def get_due_dates(request, course_key, user):
@@ -1113,25 +1110,32 @@ class ProgramCourseEnrollmentOverviewView(DeveloperErrorViewMixin, ProgramSpecif
"""
if is_bulk_email_feature_enabled(course_id=course_id):
return not is_user_opted_out_for_course(user=user, course_id=course_id)
else:
return None
return None
@staticmethod
def get_course_run_status(course_overview, is_certificate_passing, certificate_creation_date):
def get_course_run_status(course_overview, certificate_info):
"""
Get the progress status of a course run.
Get the progress status of a course run, given the state of a user's certificate in the course.
In the case of self-paced course runs, the run is considered completed when either the course run has ended
OR the user has earned a passing certificate 30 days ago or longer.
Arguments:
course_overview (CourseOverview): the overview for the course run
is_certificate_passing (bool): True if the user has a passing certificate in
this course run; False otherwise
certificate_creation_date: the date the certificate was created
certificate_info: A dict containing the following keys:
``is_passing``: whether the user has a passing certificate in the course run
``created``: the date the certificate was created
Returns:
status: one of CourseRunProgressStatuses.COMPLETE,
status: one of (
CourseRunProgressStatuses.COMPLETE,
CourseRunProgressStatuses.IN_PROGRESS,
or CourseRunProgressStatuses.UPCOMING
CourseRunProgressStatuses.UPCOMING,
)
"""
is_certificate_passing = certificate_info.get('is_passing', False)
certificate_creation_date = certificate_info.get('created', datetime.max)
if course_overview.pacing == 'instructor':
if course_overview.has_ended():
return CourseRunProgressStatuses.COMPLETED
@@ -1140,11 +1144,9 @@ class ProgramCourseEnrollmentOverviewView(DeveloperErrorViewMixin, ProgramSpecif
else:
return CourseRunProgressStatuses.UPCOMING
elif course_overview.pacing == 'self':
has_ended = course_overview.has_ended()
thirty_days_ago = datetime.now(UTC) - timedelta(30)
# a self paced course run is completed when either the course run has ended
# OR the user has earned a certificate 30 days ago or more
if has_ended or is_certificate_passing and (certificate_creation_date and certificate_creation_date <= thirty_days_ago):
certificate_completed = is_certificate_passing and (certificate_creation_date <= thirty_days_ago)
if course_overview.has_ended() or certificate_completed:
return CourseRunProgressStatuses.COMPLETED
elif course_overview.has_started():
return CourseRunProgressStatuses.IN_PROGRESS

View File

@@ -17,7 +17,7 @@ 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.catalog.utils import create_catalog_api_client
from openedx.core.djangoapps.catalog.utils import create_catalog_api_client, course_run_keys_for_program
logger = logging.getLogger(__name__)
User = get_user_model() # pylint: disable=invalid-name
@@ -218,8 +218,7 @@ class Command(BaseCommand):
failure = False
for program in programs.values():
for course in program['courses']:
for course_run in course['course_runs']:
course_run_cache_key = COURSE_PROGRAMS_CACHE_KEY_TPL.format(course_run_id=course_run['key'])
course_runs[course_run_cache_key].append(program['uuid'])
for course_run_key in course_run_keys_for_program(program):
course_run_cache_key = COURSE_PROGRAMS_CACHE_KEY_TPL.format(course_run_id=course_run_key)
course_runs[course_run_cache_key].append(program['uuid'])
return course_runs, failure

View File

@@ -9,6 +9,7 @@ from django.core.cache import cache
from django.core.management import call_command
from openedx.core.djangoapps.catalog.cache import (
COURSE_PROGRAMS_CACHE_KEY_TPL,
PATHWAY_CACHE_KEY_TPL,
PROGRAM_CACHE_KEY_TPL,
SITE_PATHWAY_IDS_CACHE_KEY_TPL,
@@ -24,6 +25,9 @@ from student.tests.factories import UserFactory
@skip_unless_lms
@httpretty.activate
class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMixin):
"""
Defines tests for the ``cache_programs`` management command.
"""
ENABLED_CACHES = ['default']
def setUp(self):
@@ -46,6 +50,10 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
self.programs = ProgramFactory.create_batch(3)
self.pathways = PathwayFactory.create_batch(3)
self.child_program = ProgramFactory.create()
self.programs[0]['curricula'][0]['programs'].append(self.child_program)
self.programs.append(self.child_program)
for pathway in self.pathways:
self.programs += pathway['programs']
@@ -57,7 +65,10 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
self.pathways[1]['programs'].append(self.programs[0])
def mock_list(self):
""" Mock the data returned by the program listing API endpoint. """
# pylint: disable=unused-argument
def list_callback(request, uri, headers):
""" The mock listing callback. """
expected = {
'exclude_utm': ['1'],
'status': ['active', 'retired'],
@@ -75,7 +86,10 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
)
def mock_detail(self, uuid, program):
""" Mock the data returned by the program detail API endpoint. """
# pylint: disable=unused-argument
def detail_callback(request, uri, headers):
""" The mock detail callback. """
expected = {
'exclude_utm': ['1'],
}
@@ -91,13 +105,9 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
)
def mock_pathways(self, pathways, page_number=1, final=True):
"""
Mock the data for discovery's credit pathways endpoint
"""
""" Mock the data for discovery's credit pathways endpoint. """
def pathways_callback(request, uri, headers): # pylint: disable=unused-argument
"""
Mocks response
"""
""" Mocks the pathways response. """
expected = {
'exclude_utm': ['1'],
@@ -171,6 +181,15 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
del program['pathway_ids']
self.assertEqual(program, programs[key])
# the courses in the child program's first curriculum (the active one)
# should point to both the child program and the first program
# in the cache.
for course in self.child_program['curricula'][0]['courses']:
for course_run in course['course_runs']:
course_run_cache_key = COURSE_PROGRAMS_CACHE_KEY_TPL.format(course_run_id=course_run['key'])
self.assertIn(self.programs[0]['uuid'], cache.get(course_run_cache_key))
self.assertIn(self.child_program['uuid'], cache.get(course_run_cache_key))
def test_handle_pathways(self):
"""
Verify that the command requests and caches credit pathways

View File

@@ -197,7 +197,7 @@ def generate_curricula():
class ProgramFactory(DictFactoryBase):
authoring_organizations = factory.LazyFunction(partial(generate_instances, OrganizationFactory, count=1))
applicable_seat_types = []
applicable_seat_types = factory.LazyFunction(lambda: [])
banner_image = factory.LazyFunction(generate_sized_stdimage)
card_image_url = factory.Faker('image_url')
corporate_endorsements = factory.LazyFunction(partial(generate_instances, CorporateEndorsementFactory))
@@ -232,7 +232,7 @@ class CurriculumFactory(DictFactoryBase):
marketing_text_brief = factory.Faker('word')
is_active = True
courses = factory.LazyFunction(partial(generate_instances, CourseFactory))
programs = []
programs = factory.LazyFunction(lambda: [])
class ProgramTypeFactory(DictFactoryBase):

View File

@@ -35,6 +35,8 @@ from openedx.core.djangoapps.catalog.tests.factories import (
)
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
from openedx.core.djangoapps.catalog.utils import (
child_programs,
course_run_keys_for_program,
get_course_run_details,
get_course_runs,
get_course_runs_for_course,
@@ -663,3 +665,123 @@ class TestGetCourseRunDetails(CatalogIntegrationMixin, TestCase):
data = get_course_run_details(course_run['key'], ['content_language', 'weeks_to_complete', 'max_effort'])
self.assertTrue(mock_get_edx_api_data.called)
self.assertEqual(data, course_run_details)
class TestProgramCourseRunCrawling(TestCase):
@classmethod
def setUpClass(cls):
super(TestProgramCourseRunCrawling, cls).setUpClass()
cls.grandchild_1 = {
'title': 'grandchild 1',
'curricula': [{'is_active': True, 'courses': [], 'programs': []}],
}
cls.grandchild_2 = {
'title': 'grandchild 2',
'curricula': [
{
'is_active': True,
'courses': [{
'course_runs': [
{'key': 'course-run-4'},
],
}],
'programs': [],
},
],
}
cls.grandchild_3 = {
'title': 'grandchild 3',
'curricula': [{'is_active': False}],
}
cls.child_1 = {
'title': 'child 1',
'curricula': [{'is_active': True, 'courses': [], 'programs': [cls.grandchild_1]}],
}
cls.child_2 = {
'title': 'child 2',
'curricula': [
{
'is_active': True,
'courses': [{
'course_runs': [
{'key': 'course-run-3'},
],
}],
'programs': [cls.grandchild_2, cls.grandchild_3],
},
],
}
cls.complex_program = {
'title': 'complex program',
'curricula': [
{
'is_active': True,
'courses': [{
'course_runs': [
{'key': 'course-run-2'},
],
}],
'programs': [cls.child_1, cls.child_2],
},
],
}
cls.simple_program = {
'title': 'simple program',
'curricula': [
{
'is_active': True,
'courses': [{
'course_runs': [
{'key': 'course-run-1'},
],
}],
'programs': [cls.grandchild_1]
},
],
}
cls.empty_program = {
'title': 'notice that I have a curriculum, but no programs inside it',
'curricula': [
{
'is_active': True,
'courses': [],
'programs': [],
},
],
}
def test_child_programs_no_curriculum(self):
program = {
'title': 'notice that I do not have a curriculum',
}
self.assertEqual([], child_programs(program))
def test_child_programs_no_children(self):
self.assertEqual([], child_programs(self.empty_program))
def test_child_programs_one_child(self):
self.assertEqual([self.grandchild_1], child_programs(self.simple_program))
def test_child_programs_many_children(self):
expected_children = [
self.child_1,
self.grandchild_1,
self.child_2,
self.grandchild_2,
self.grandchild_3,
]
self.assertEqual(expected_children, child_programs(self.complex_program))
def test_course_run_keys_for_program_no_courses(self):
self.assertEqual(set(), course_run_keys_for_program(self.empty_program))
def test_course_run_keys_for_program_one_course(self):
self.assertEqual({'course-run-1'}, course_run_keys_for_program(self.simple_program))
def test_course_run_keys_for_program_many_courses(self):
expected_course_runs = {
'course-run-2',
'course-run-3',
'course-run-4',
}
self.assertEqual(expected_course_runs, course_run_keys_for_program(self.complex_program))

View File

@@ -543,3 +543,56 @@ def get_course_run_details(course_run_key, fields):
course_run_details = get_edx_api_data(catalog_integration, 'course_runs', api, resource_id=course_run_key,
cache_key=cache_key, many=False, traverse_pagination=False, fields=fields)
return course_run_details
def course_run_keys_for_program(parent_program):
"""
All of the course run keys associated with this ``parent_program``, either
via its ``curriculum`` field (looking at both the curriculum's courses
and child programs), or through the many-to-many ``courses`` field on the program.
"""
keys = set()
for program in [parent_program] + child_programs(parent_program):
curriculum = _primary_active_curriculum(program)
if curriculum:
keys.update(_course_runs_from_container(curriculum))
keys.update(_course_runs_from_container(program))
return keys
def child_programs(program):
"""
Given a program, recursively find all child programs related
to this program through its curricula.
"""
curriculum = _primary_active_curriculum(program)
if not curriculum:
return []
result = []
for child in curriculum.get('programs', []):
result.append(child)
result.extend(child_programs(child))
return result
def _primary_active_curriculum(program):
"""
Returns the first active curriculum in the given program, or None.
"""
try:
return next(c for c in program.get('curricula', []) if c.get('is_active'))
except StopIteration:
return
def _course_runs_from_container(container):
"""
Pluck nested course runs out of a ``container`` dictionary,
which is either the ``curriculum`` field of a program, or
a program itself (since either may contain a ``courses`` list).
"""
return [
course_run.get('key')
for course in container.get('courses', [])
for course_run in course.get('course_runs', [])
]