Skip to content

E
edx-platform
Commit 8289c321 by Nimisha Asthagiri
Options

Update CourseOverview for Course Catalog 

Added the following fields to CourseOverview:
    announcement
    catalog_visibility
    course_video_url
    effort
    short_description

Now requires CourseOverview models for each course in the system:
    Introduced a CourseOverviewGeneratedHistory model to keep track of
    when the CourseOverview table was seeded. The seeding can be
    done preemptively by calling the generate_course_overview management
    command. Otherwise, it is lazily computed, when needed.
parent 4334eb9b
Showing with 327 additions and 49 deletions
openedx/core/djangoapps/content/course_overviews/management/commands/generate_course_overview.py
......@@ -32,11 +32,13 @@ class Command(BaseCommand):
)
def handle(self, *args, **options):
course_keys = []
if options['all']:
course_keys = [course.id for course in modulestore().get_courses()]
# Have CourseOverview generate course overviews for all
# the courses in the system.
CourseOverview.get_all_courses(force_reseeding=True)
else:
course_keys = []
if len(args) < 1:
raise CommandError('At least one course or --all must be specified.')
try:
......@@ -44,17 +46,7 @@ class Command(BaseCommand):
except InvalidKeyError:
log.fatal('Invalid key specified.')
if not course_keys:
log.fatal('No courses specified.')
if not course_keys:
log.fatal('No courses specified.')
log.info('Generating course overview for %d courses.', len(course_keys))
log.debug('Generating course overview(s) for the following courses: %s', course_keys)
for course_key in course_keys:
try:
CourseOverview.get_from_id(course_key)
except Exception as ex: # pylint: disable=broad-except
log.exception('An error occurred while generating course overview for %s: %s', unicode(
course_key), ex.message)
log.info('Finished generating course overviews.')
CourseOverview.get_select_courses(course_keys)
openedx/core/djangoapps/content/course_overviews/management/commands/tests/test_generate_course_overview.py
......@@ -64,7 +64,7 @@ class TestGenerateCourseOverview(ModuleStoreTestCase):
self.command.handle('not/found', all=False)
self.assertTrue(mock_log.fatal.called)
@patch('openedx.core.djangoapps.content.course_overviews.management.commands.generate_course_overview.log')
@patch('openedx.core.djangoapps.content.course_overviews.models.log')
def test_not_found_key(self, mock_log):
"""
Test keys not found are logged.
......
openedx/core/djangoapps/content/course_overviews/migrations/0002_add_course_catalog_fields.py 0 → 100644
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('course_overviews', '0001_initial'),
]
operations = [
migrations.AddField(
model_name='courseoverview',
name='announcement',
field=models.DateTimeField(null=True),
),
migrations.AddField(
model_name='courseoverview',
name='catalog_visibility',
field=models.TextField(null=True),
),
migrations.AddField(
model_name='courseoverview',
name='course_video_url',
field=models.TextField(null=True),
),
migrations.AddField(
model_name='courseoverview',
name='effort',
field=models.TextField(null=True),
),
migrations.AddField(
model_name='courseoverview',
name='short_description',
field=models.TextField(null=True),
),
]
openedx/core/djangoapps/content/course_overviews/migrations/0003_courseoverviewgeneratedhistory.py 0 → 100644
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import migrations, models
import django.utils.timezone
import model_utils.fields
class Migration(migrations.Migration):
dependencies = [
('course_overviews', '0002_add_course_catalog_fields'),
]
operations = [
migrations.CreateModel(
name='CourseOverviewGeneratedHistory',
fields=[
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, verbose_name='created', editable=False)),
('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, verbose_name='modified', editable=False)),
('num_courses', models.IntegerField(null=True)),
],
options={
'abstract': False,
},
),
]
openedx/core/djangoapps/content/course_overviews/migrations/0004_courseoverview_org.py 0 → 100644
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('course_overviews', '0003_courseoverviewgeneratedhistory'),
]
operations = [
migrations.AddField(
model_name='courseoverview',
name='org',
field=models.TextField(default=b'outdated_entry', max_length=255),
),
]
openedx/core/djangoapps/content/course_overviews/models.py
......@@ -2,19 +2,21 @@
Declaration of CourseOverview model
"""
import json
from django.db import models, transaction
import logging
from django.db import models, transaction
from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField, IntegerField
from django.db.utils import IntegrityError
from django.template import defaultfilters
from django.utils.translation import ugettext
from lms.djangoapps import django_comment_client
from model_utils.models import TimeStampedModel
from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.models.course_details import CourseDetails
from util.date_utils import strftime_localized
from xmodule import course_metadata_utils
from xmodule.course_module import CourseDescriptor
from xmodule.course_module import CourseDescriptor, DEFAULT_START_DATE
from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore.django import modulestore
from xmodule_django.models import CourseKeyField, UsageKeyField
......@@ -22,20 +24,26 @@ from xmodule_django.models import CourseKeyField, UsageKeyField
from ccx_keys.locator import CCXLocator
log = logging.getLogger(__name__)
class CourseOverview(TimeStampedModel):
"""
Model for storing and caching basic information about a course.
This model contains basic course metadata such as an ID, display name,
image URL, and any other information that would be necessary to display
a course as part of a user dashboard or enrollment API.
a course as part of:
user dashboard (enrolled courses)
course catalog (courses to enroll in)
course about (meta data about the course)
"""
class Meta(object):
app_label = 'course_overviews'
# IMPORTANT: Bump this whenever you modify this model and/or add a migration.
VERSION = 2
VERSION = 3
# Cache entry versioning.
version = IntegerField()
......@@ -43,6 +51,7 @@ class CourseOverview(TimeStampedModel):
# Course identification
id = CourseKeyField(db_index=True, primary_key=True, max_length=255)
_location = UsageKeyField(max_length=255)
org = TextField(max_length=255, default='outdated_entry')
display_name = TextField(null=True)
display_number_with_default = TextField()
display_org_with_default = TextField()
......@@ -51,6 +60,7 @@ class CourseOverview(TimeStampedModel):
start = DateTimeField(null=True)
end = DateTimeField(null=True)
advertised_start = TextField(null=True)
announcement = DateTimeField(null=True)
# URLs
course_image_url = TextField()
......@@ -82,6 +92,12 @@ class CourseOverview(TimeStampedModel):
invitation_only = BooleanField(default=False)
max_student_enrollments_allowed = IntegerField(null=True)
# Catalog information
catalog_visibility = TextField(null=True)
short_description = TextField(null=True)
course_video_url = TextField(null=True)
effort = TextField(null=True)
@classmethod
def _create_from_course(cls, course):
"""
......@@ -99,6 +115,8 @@ class CourseOverview(TimeStampedModel):
from lms.djangoapps.certificates.api import get_active_web_certificate
from openedx.core.lib.courses import course_image_url
log.info('Creating course overview for %s.', unicode(course.id))
# Workaround for a problem discovered in https://openedx.atlassian.net/browse/TNL-2806.
# If the course has a malformed grading policy such that
# course._grading_policy['GRADE_CUTOFFS'] = {}, then
......@@ -125,6 +143,7 @@ class CourseOverview(TimeStampedModel):
version=cls.VERSION,
id=course.id,
_location=course.location,
org=course.location.org,
display_name=display_name,
display_number_with_default=course.display_number_with_default,
display_org_with_default=course.display_org_with_default,
......@@ -132,6 +151,7 @@ class CourseOverview(TimeStampedModel):
start=start,
end=end,
advertised_start=course.advertised_start,
announcement=course.announcement,
course_image_url=course_image_url(course),
facebook_url=course.facebook_url,
......@@ -156,6 +176,11 @@ class CourseOverview(TimeStampedModel):
enrollment_domain=course.enrollment_domain,
invitation_only=course.invitation_only,
max_student_enrollments_allowed=max_student_enrollments_allowed,
catalog_visibility=course.catalog_visibility,
short_description=CourseDetails.fetch_about_attribute(course.id, 'short_description'),
effort=CourseDetails.fetch_about_attribute(course.id, 'effort'),
course_video_url=CourseDetails.fetch_video_url(course.id),
)
@classmethod
......@@ -343,6 +368,42 @@ class CourseOverview(TimeStampedModel):
strftime_localized
)
@property
def sorting_score(self):
"""
Returns a tuple that can be used to sort the courses according
the how "new" they are. The "newness" score is computed using a
heuristic that takes into account the announcement and
(advertised) start dates of the course if available.
The lower the number the "newer" the course.
"""
return course_metadata_utils.sorting_score(self.start, self.advertised_start, self.announcement)
@property
def start_type(self):
"""
Returns the type of the course's 'start' field.
"""
if self.advertised_start:
return u'string'
elif self.start != DEFAULT_START_DATE:
return u'timestamp'
else:
return u'empty'
@property
def start_display(self):
"""
Returns the display value for the course's start date.
"""
if self.advertised_start:
return self.advertised_start
elif self.start != DEFAULT_START_DATE:
return defaultfilters.date(self.start, "DATE_FORMAT")
else:
return None
def may_certify(self):
"""
Returns whether it is acceptable to show the student a certificate
......@@ -362,6 +423,72 @@ class CourseOverview(TimeStampedModel):
return json.loads(self._pre_requisite_courses_json)
@classmethod
def get_select_courses(cls, course_keys):
"""
Returns CourseOverview objects for the given course_keys.
"""
course_overviews = []
log.info('Generating course overview for %d courses.', len(course_keys))
log.debug('Generating course overview(s) for the following courses: %s', course_keys)
for course_key in course_keys:
try:
course_overviews.append(CourseOverview.get_from_id(course_key))
except Exception as ex: # pylint: disable=broad-except
log.exception(
'An error occurred while generating course overview for %s: %s',
unicode(course_key),
ex.message,
)
log.info('Finished generating course overviews.')
return course_overviews
@classmethod
def get_all_courses(cls, force_reseeding=False, org=None):
"""
Returns all CourseOverview objects in the database.
Arguments:
force_reseeding (bool): Optional parameter.
If True, the modulestore is used as the source of truth for
the list of courses, even if the CourseOverview table was
previously seeded. However, only non-existing CourseOverview
entries or those with older data model versions or will get
populated.
If False, the list of courses is retrieved from the
CourseOverview table if it was previously seeded, falling
back to the modulestore if it wasn't seeded.
org (string): Optional parameter that allows filtering
by organization.
"""
if force_reseeding or not CourseOverviewGeneratedHistory.objects.first():
# Seed the CourseOverview table with data for all
# courses in the system.
course_keys = [course.id for course in modulestore().get_courses()]
course_overviews = cls.get_select_courses(course_keys)
num_courses = len(course_overviews)
CourseOverviewGeneratedHistory.objects.create(num_courses=num_courses)
if org:
course_overviews = [c for c in course_overviews if c.org == org]
else:
# Note: If a newly created course is not returned in this QueryList,
# make sure the "publish" signal was emitted when the course was
# created. For tests using CourseFactory, use emit_signals=True.
# Or pass True for force_reseeding.
course_overviews = CourseOverview.objects.all()
if org:
course_overviews = course_overviews.filter(org=org)
return course_overviews
@classmethod
def get_all_course_keys(cls):
"""
Returns all course keys from course overviews.
......@@ -389,3 +516,14 @@ class CourseOverviewTab(models.Model):
"""
tab_id = models.CharField(max_length=50)
course_overview = models.ForeignKey(CourseOverview, db_index=True, related_name="tabs")
class CourseOverviewGeneratedHistory(TimeStampedModel):
"""
Model for keeping track of when CourseOverview Models are
generated/seeded.
"""
num_courses = IntegerField(null=True)
def __unicode__(self):
return self.num_courses
openedx/core/djangoapps/content/course_overviews/tests.py
......@@ -11,8 +11,14 @@ import pytz
from django.utils import timezone
from lms.djangoapps.certificates.api import get_active_web_certificate
from openedx.core.djangoapps.models.course_details import CourseDetails
from openedx.core.lib.courses import course_image_url
from xmodule.course_metadata_utils import DEFAULT_START_DATE
from xmodule.course_module import (
CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
CATALOG_VISIBILITY_ABOUT,
CATALOG_VISIBILITY_NONE,
)
from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
......@@ -96,6 +102,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
'enrollment_domain',
'invitation_only',
'max_student_enrollments_allowed',
'catalog_visibility',
]
for attribute_name in fields_to_test:
course_value = getattr(course, attribute_name)
......@@ -124,45 +131,49 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
self.assertEqual(cache_miss_value, cache_hit_value)
# Other values to test
# Note: we test the start and end attributes here instead of in
# fields_to_test, because I ran into trouble while testing datetimes
# Note: we test the time-related attributes here instead of in
# fields_to_test, because we run into trouble while testing datetimes
# for equality. When writing and reading dates from databases, the
# resulting values are often off by fractions of a second. So, as a
# workaround, we simply test if the start and end times are the same
# number of seconds from the Unix epoch.
time_field_accessor = lambda object, field_name: get_seconds_since_epoch(getattr(object, field_name))
# The course about fields are accessed through the CourseDetail
# class for the course module, and stored as attributes on the
# CourseOverview objects.
course_about_accessor = lambda object, field_name: CourseDetails.fetch_about_attribute(object.id, field_name)
others_to_test = [
('start', time_field_accessor, time_field_accessor),
('end', time_field_accessor, time_field_accessor),
('enrollment_start', time_field_accessor, time_field_accessor),
('enrollment_end', time_field_accessor, time_field_accessor),
('announcement', time_field_accessor, time_field_accessor),
('short_description', course_about_accessor, getattr),
('effort', course_about_accessor, getattr),
(
course_image_url(course),
course_overview_cache_miss.course_image_url,
course_overview_cache_hit.course_image_url
),
(
get_active_web_certificate(course) is not None,
course_overview_cache_miss.has_any_active_web_certificate,
course_overview_cache_hit.has_any_active_web_certificate
),
(
get_seconds_since_epoch(course.start),
get_seconds_since_epoch(course_overview_cache_miss.start),
get_seconds_since_epoch(course_overview_cache_hit.start),
),
(
get_seconds_since_epoch(course.end),
get_seconds_since_epoch(course_overview_cache_miss.end),
get_seconds_since_epoch(course_overview_cache_hit.end),
'video',
lambda c, __: CourseDetails.fetch_video_url(c.id),
lambda c, __: c.course_video_url,
),
(
get_seconds_since_epoch(course.enrollment_start),
get_seconds_since_epoch(course_overview_cache_miss.enrollment_start),
get_seconds_since_epoch(course_overview_cache_hit.enrollment_start),
'course_image_url',
lambda c, __: course_image_url(c),
getattr,
),
(
get_seconds_since_epoch(course.enrollment_end),
get_seconds_since_epoch(course_overview_cache_miss.enrollment_end),
get_seconds_since_epoch(course_overview_cache_hit.enrollment_end),
'has_any_active_web_certificate',
lambda c, field_name: get_active_web_certificate(c) is not None,
getattr,
),
]
for (course_value, cache_miss_value, cache_hit_value) in others_to_test:
for attribute_name, course_accessor, course_overview_accessor in others_to_test:
course_value = course_accessor(course, attribute_name)
cache_miss_value = course_overview_accessor(course_overview_cache_miss, attribute_name)
cache_hit_value = course_overview_accessor(course_overview_cache_hit, attribute_name)
self.assertEqual(course_value, cache_miss_value)
self.assertEqual(cache_miss_value, cache_hit_value)
......@@ -178,6 +189,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
"display_name": "Test Course", # Display name provided
"start": LAST_WEEK, # In the middle of the course
"end": NEXT_WEEK,
"announcement": LAST_MONTH, # Announcement date provided
"advertised_start": "2015-01-01 11:22:33", # Parse-able advertised_start
"pre_requisite_courses": [ # Has pre-requisites
'course-v1://edX+test1+run1',
......@@ -194,6 +206,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
"pre_requisite_courses": [], # No pre-requisites
"static_asset_path": "my/relative/path", # Relative asset path
"certificates_show_before_end": False,
"catalog_visibility": CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
},
{
"display_name": "", # Empty display name
......@@ -203,6 +216,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
"pre_requisite_courses": [], # No pre-requisites
"static_asset_path": "", # Empty asset path
"certificates_show_before_end": False,
"catalog_visibility": CATALOG_VISIBILITY_ABOUT,
},
{
# # Don't set display name
......@@ -212,6 +226,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
"pre_requisite_courses": [], # No pre-requisites
"static_asset_path": None, # No asset path
"certificates_show_before_end": False,
"catalog_visibility": CATALOG_VISIBILITY_NONE,
}
],
[ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split]
......@@ -325,7 +340,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
course_overview = CourseOverview._create_from_course(course) # pylint: disable=protected-access
self.assertEqual(course_overview.lowest_passing_grade, None)
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 1), (ModuleStoreEnum.Type.split, 3, 4))
@ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 3, 4))
@ddt.unpack
def test_versioning(self, modulestore_type, min_mongo_calls, max_mongo_calls):
"""
......@@ -425,3 +440,50 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
# knows how to write, it's not going to overwrite what's there.
unmodified_overview = CourseOverview.get_from_id(course.id)
self.assertEqual(unmodified_overview.version, 11)
def test_get_select_courses(self):
course_ids = [CourseFactory.create().id for __ in range(3)]
select_course_ids = course_ids[:len(course_ids) - 1] # all items except the last
self.assertSetEqual(
{course_overview.id for course_overview in CourseOverview.get_select_courses(select_course_ids)},
set(select_course_ids),
)
def test_get_all_courses(self):
course_ids = [CourseFactory.create().id for __ in range(3)]
self.assertSetEqual(
{course_overview.id for course_overview in CourseOverview.get_all_courses()},
set(course_ids),
)
with mock.patch(
'openedx.core.djangoapps.content.course_overviews.models.CourseOverview.get_from_id'
) as mock_get_from_id:
CourseOverview.get_all_courses()
self.assertFalse(mock_get_from_id.called)
CourseOverview.get_all_courses(force_reseeding=True)
self.assertTrue(mock_get_from_id.called)
def test_get_all_courses_by_org(self):
org_courses = [] # list of lists of courses
for index in range(2):
org_courses.append([
CourseFactory.create(org='test_org_' + unicode(index))
for __ in range(3)
])
self.assertSetEqual(
{c.id for c in CourseOverview.get_all_courses(org='test_org_0', force_reseeding=True)},
{c.id for c in org_courses[0]},
)
self.assertSetEqual(
{c.id for c in CourseOverview.get_all_courses(org='test_org_1')},
{c.id for c in org_courses[1]},
)
self.assertSetEqual(
{c.id for c in CourseOverview.get_all_courses()},
{c.id for c in org_courses[0] + org_courses[1]},
)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment