diff --git a/cms/envs/common.py b/cms/envs/common.py index 0057b86f229dad64d924d02e88b01fb583050522..084eb4103cef54e9735d4662694e624d25e82941 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -754,6 +754,7 @@ INSTALLED_APPS = ( # Additional problem types 'edx_jsme', # Molecular Structure + 'openedx.core.djangoapps.content.course_overviews', 'openedx.core.djangoapps.content.course_structures', # Credit courses diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 18c4bcc550b177f4f62f471a8b3d087378ed4a73..5335b73e7e9880e896e95269088548729dfa8285 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1315,6 +1315,14 @@ class CourseEnrollment(models.Model): def course(self): return modulestore().get_course(self.course_id) + @property + def course_overview(self): + """ + Return a CourseOverview of this enrollment's course. + """ + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + return CourseOverview.get_from_id(self.course_id) + def is_verified_enrollment(self): """ Check the course enrollment mode is verified or not diff --git a/common/lib/xmodule/xmodule/course_metadata_utils.py b/common/lib/xmodule/xmodule/course_metadata_utils.py new file mode 100644 index 0000000000000000000000000000000000000000..07e6d384cb5f5d046d33c6f9f0b67ee7d7310b5c --- /dev/null +++ b/common/lib/xmodule/xmodule/course_metadata_utils.py @@ -0,0 +1,210 @@ +""" +Simple utility functions that operate on course metadata. + +This is a place to put simple functions that operate on course metadata. It +allows us to share code between the CourseDescriptor and CourseOverview +classes, which both need these type of functions. +""" +from datetime import datetime +from base64 import b32encode + +from django.utils.timezone import UTC + +from .fields import Date + +DEFAULT_START_DATE = datetime(2030, 1, 1, tzinfo=UTC()) + + +def clean_course_key(course_key, padding_char): + """ + Encode a course's key into a unique, deterministic base32-encoded ID for + the course. + + Arguments: + course_key (CourseKey): A course key. + padding_char (str): Character used for padding at end of the encoded + string. The standard value for this is '='. + """ + return "course_{}".format( + b32encode(unicode(course_key)).replace('=', padding_char) + ) + + +def url_name_for_course_location(location): + """ + Given a course's usage locator, returns the course's URL name. + + Arguments: + location (BlockUsageLocator): The course's usage locator. + """ + return location.name + + +def display_name_with_default(course): + """ + Calculates the display name for a course. + + Default to the display_name if it isn't None, else fall back to creating + a name based on the URL. + + Unlike the rest of this module's functions, this function takes an entire + course descriptor/overview as a parameter. This is because a few test cases + (specifically, {Text|Image|Video}AnnotationModuleTestCase.test_student_view) + create scenarios where course.display_name is not None but course.location + is None, which causes calling course.url_name to fail. So, although we'd + like to just pass course.display_name and course.url_name as arguments to + this function, we can't do so without breaking those tests. + + Arguments: + course (CourseDescriptor|CourseOverview): descriptor or overview of + said course. + """ + # TODO: Consider changing this to use something like xml.sax.saxutils.escape + return ( + course.display_name if course.display_name is not None + else course.url_name.replace('_', ' ') + ).replace('<', '<').replace('>', '>') + + +def number_for_course_location(location): + """ + Given a course's block usage locator, returns the course's number. + + This is a "number" in the sense of the "course numbers" that you see at + lots of universities. For example, given a course + "Intro to Computer Science" with the course key "edX/CS-101/2014", the + course number would be "CS-101" + + Arguments: + location (BlockUsageLocator): The usage locator of the course in + question. + """ + return location.course + + +def has_course_started(start_date): + """ + Given a course's start datetime, returns whether the current time's past it. + + Arguments: + start_date (datetime): The start datetime of the course in question. + """ + # TODO: This will throw if start_date is None... consider changing this behavior? + return datetime.now(UTC()) > start_date + + +def has_course_ended(end_date): + """ + Given a course's end datetime, returns whether + (a) it is not None, and + (b) the current time is past it. + + Arguments: + end_date (datetime): The end datetime of the course in question. + """ + return datetime.now(UTC()) > end_date if end_date is not None else False + + +def course_start_date_is_default(start, advertised_start): + """ + Returns whether a course's start date hasn't yet been set. + + Arguments: + start (datetime): The start datetime of the course in question. + advertised_start (str): The advertised start date of the course + in question. + """ + return advertised_start is None and start == DEFAULT_START_DATE + + +def _datetime_to_string(date_time, format_string, strftime_localized): + """ + Formats the given datetime with the given function and format string. + + Adds UTC to the resulting string if the format is DATE_TIME or TIME. + + Arguments: + date_time (datetime): the datetime to be formatted + format_string (str): the date format type, as passed to strftime + strftime_localized ((datetime, str) -> str): a nm localized string + formatting function + """ + # TODO: Is manually appending UTC really the right thing to do here? What if date_time isn't UTC? + result = strftime_localized(date_time, format_string) + return ( + result + u" UTC" if format_string in ['DATE_TIME', 'TIME'] + else result + ) + + +def course_start_datetime_text(start_date, advertised_start, format_string, ugettext, strftime_localized): + """ + Calculates text to be shown to user regarding a course's start + datetime in UTC. + + Prefers .advertised_start, then falls back to .start. + + Arguments: + start_date (datetime): the course's start datetime + advertised_start (str): the course's advertised start date + format_string (str): the date format type, as passed to strftime + ugettext ((str) -> str): a text localization function + strftime_localized ((datetime, str) -> str): a localized string + formatting function + """ + if advertised_start is not None: + # TODO: This will return an empty string if advertised_start == ""... consider changing this behavior? + try: + # from_json either returns a Date, returns None, or raises a ValueError + parsed_advertised_start = Date().from_json(advertised_start) + except ValueError: + parsed_advertised_start = None + return ( + _datetime_to_string(parsed_advertised_start, format_string, strftime_localized) if parsed_advertised_start + else advertised_start.title() + ) + elif start_date != DEFAULT_START_DATE: + return _datetime_to_string(start_date, format_string, strftime_localized) + else: + _ = ugettext + # Translators: TBD stands for 'To Be Determined' and is used when a course + # does not yet have an announced start date. + return _('TBD') + + +def course_end_datetime_text(end_date, format_string, strftime_localized): + """ + Returns a formatted string for a course's end date or datetime. + + If end_date is None, an empty string will be returned. + + Arguments: + end_date (datetime): the end datetime of a course + format_string (str): the date format type, as passed to strftime + strftime_localized ((datetime, str) -> str): a localized string + formatting function + """ + return ( + _datetime_to_string(end_date, format_string, strftime_localized) if end_date is not None + else '' + ) + + +def may_certify_for_course(certificates_display_behavior, certificates_show_before_end, has_ended): + """ + Returns whether it is acceptable to show the student a certificate download + link for a course. + + Arguments: + certificates_display_behavior (str): string describing the course's + certificate display behavior. + See CourseFields.certificates_display_behavior.help for more detail. + certificates_show_before_end (bool): whether user can download the + course's certificates before the course has ended. + has_ended (bool): Whether the course has ended. + """ + show_early = ( + certificates_display_behavior in ('early_with_info', 'early_no_info') + or certificates_show_before_end + ) + return show_early or has_ended diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index a0334fdd896ff0491d5dc1b0dc2518124967eea0..832791ba5991c58e34c0649c1fac648c58e9b734 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -10,8 +10,9 @@ import requests from datetime import datetime import dateutil.parser from lazy import lazy -from base64 import b32encode +from xmodule import course_metadata_utils +from xmodule.course_metadata_utils import DEFAULT_START_DATE from xmodule.exceptions import UndefinedContext from xmodule.seq_module import SequenceDescriptor, SequenceModule from xmodule.graders import grader_from_conf @@ -29,8 +30,6 @@ log = logging.getLogger(__name__) # Make '_' a no-op so we can scrape strings _ = lambda text: text -DEFAULT_START_DATE = datetime(2030, 1, 1, tzinfo=UTC()) - CATALOG_VISIBILITY_CATALOG_AND_ABOUT = "both" CATALOG_VISIBILITY_ABOUT = "about" CATALOG_VISIBILITY_NONE = "none" @@ -1089,20 +1088,20 @@ class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): Returns True if the current time is after the specified course end date. Returns False if there is no end date specified. """ - if self.end is None: - return False - - return datetime.now(UTC()) > self.end + return course_metadata_utils.has_course_ended(self.end) def may_certify(self): """ - Return True if it is acceptable to show the student a certificate download link + Return whether it is acceptable to show the student a certificate download link. """ - show_early = self.certificates_display_behavior in ('early_with_info', 'early_no_info') or self.certificates_show_before_end - return show_early or self.has_ended() + return course_metadata_utils.may_certify_for_course( + self.certificates_display_behavior, + self.certificates_show_before_end, + self.has_ended() + ) def has_started(self): - return datetime.now(UTC()) > self.start + return course_metadata_utils.has_course_started(self.start) @property def grader(self): @@ -1361,36 +1360,13 @@ class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): then falls back to .start """ i18n = self.runtime.service(self, "i18n") - _ = i18n.ugettext - strftime = i18n.strftime - - def try_parse_iso_8601(text): - try: - result = Date().from_json(text) - if result is None: - result = text.title() - else: - result = strftime(result, format_string) - if format_string == "DATE_TIME": - result = self._add_timezone_string(result) - except ValueError: - result = text.title() - - return result - - if isinstance(self.advertised_start, basestring): - return try_parse_iso_8601(self.advertised_start) - elif self.start_date_is_still_default: - # Translators: TBD stands for 'To Be Determined' and is used when a course - # does not yet have an announced start date. - return _('TBD') - else: - when = self.advertised_start or self.start - - if format_string == "DATE_TIME": - return self._add_timezone_string(strftime(when, format_string)) - - return strftime(when, format_string) + return course_metadata_utils.course_start_datetime_text( + self.start, + self.advertised_start, + format_string, + i18n.ugettext, + i18n.strftime + ) @property def start_date_is_still_default(self): @@ -1398,26 +1374,20 @@ class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): Checks if the start date set for the course is still default, i.e. .start has not been modified, and .advertised_start has not been set. """ - return self.advertised_start is None and self.start == CourseFields.start.default + return course_metadata_utils.course_start_date_is_default( + self.start, + self.advertised_start + ) def end_datetime_text(self, format_string="SHORT_DATE"): """ Returns the end date or date_time for the course formatted as a string. - - If the course does not have an end date set (course.end is None), an empty string will be returned. - """ - if self.end is None: - return '' - else: - strftime = self.runtime.service(self, "i18n").strftime - date_time = strftime(self.end, format_string) - return date_time if format_string == "SHORT_DATE" else self._add_timezone_string(date_time) - - def _add_timezone_string(self, date_time): """ - Adds 'UTC' string to the end of start/end date and time texts. - """ - return date_time + u" UTC" + return course_metadata_utils.course_end_datetime_text( + self.end, + format_string, + self.runtime.service(self, "i18n").strftime + ) def get_discussion_blackout_datetimes(self): """ @@ -1458,7 +1428,15 @@ class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): @property def number(self): - return self.location.course + """ + Returns this course's number. + + This is a "number" in the sense of the "course numbers" that you see at + lots of universities. For example, given a course + "Intro to Computer Science" with the course key "edX/CS-101/2014", the + course number would be "CS-101" + """ + return course_metadata_utils.number_for_course_location(self.location) @property def display_number_with_default(self): @@ -1499,9 +1477,7 @@ class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): Returns a unique deterministic base32-encoded ID for the course. The optional padding_char parameter allows you to override the "=" character used for padding. """ - return "course_{}".format( - b32encode(unicode(self.location.course_key)).replace('=', padding_char) - ) + return course_metadata_utils.clean_course_key(self.location.course_key, padding_char) @property def teams_enabled(self): diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 08654425b357ca44585683400203ffbea08362c3..e851f43c71136b3692ef7d4e80bcdcc05d641307 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -75,11 +75,11 @@ class SignalHandler(object): 1. We receive using the Django Signals mechanism. 2. The sender is going to be the class of the modulestore sending it. - 3. Always have **kwargs in your signal handler, as new things may be added. - 4. The thing that listens for the signal lives in process, but should do + 3. The names of your handler function's parameters *must* be "sender" and "course_key". + 4. Always have **kwargs in your signal handler, as new things may be added. + 5. The thing that listens for the signal lives in process, but should do almost no work. Its main job is to kick off the celery task that will do the actual work. - """ course_published = django.dispatch.Signal(providing_args=["course_key"]) library_updated = django.dispatch.Signal(providing_args=["library_key"]) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index f1649fc1e5df5b1cae94caa6a005c69474245b95..cea8c2ebc4916471ef48485ff4b8128dcea0828a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -393,18 +393,18 @@ def check_mongo_calls_range(max_finds=float("inf"), min_finds=0, max_sends=None, :param min_sends: If non-none, make sure number of send calls are >=min_sends """ with check_sum_of_calls( - pymongo.message, - ['query', 'get_more'], - max_finds, - min_finds, + pymongo.message, + ['query', 'get_more'], + max_finds, + min_finds, ): if max_sends is not None or min_sends is not None: with check_sum_of_calls( - pymongo.message, - # mongo < 2.6 uses insert, update, delete and _do_batched_insert. >= 2.6 _do_batched_write - ['insert', 'update', 'delete', '_do_batched_write_command', '_do_batched_insert', ], - max_sends if max_sends is not None else float("inf"), - min_sends if min_sends is not None else 0, + pymongo.message, + # mongo < 2.6 uses insert, update, delete and _do_batched_insert. >= 2.6 _do_batched_write + ['insert', 'update', 'delete', '_do_batched_write_command', '_do_batched_insert', ], + max_sends if max_sends is not None else float("inf"), + min_sends if min_sends is not None else 0, ): yield else: diff --git a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py new file mode 100644 index 0000000000000000000000000000000000000000..60317d02fddeb2ffb156a72bebefd97e5cabb8fa --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py @@ -0,0 +1,200 @@ +""" +Tests for course_metadata_utils. +""" +from collections import namedtuple +from datetime import timedelta, datetime +from unittest import TestCase + +from django.utils.timezone import UTC +from django.utils.translation import ugettext + +from xmodule.course_metadata_utils import ( + clean_course_key, + url_name_for_course_location, + display_name_with_default, + number_for_course_location, + has_course_started, + has_course_ended, + DEFAULT_START_DATE, + course_start_date_is_default, + course_start_datetime_text, + course_end_datetime_text, + may_certify_for_course, +) +from xmodule.fields import Date +from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( + MongoModulestoreBuilder, + VersioningModulestoreBuilder, + MixedModulestoreBuilder +) + + +_TODAY = datetime.now(UTC()) +_LAST_MONTH = _TODAY - timedelta(days=30) +_LAST_WEEK = _TODAY - timedelta(days=7) +_NEXT_WEEK = _TODAY + timedelta(days=7) + + +class CourseMetadataUtilsTestCase(TestCase): + """ + Tests for course_metadata_utils. + """ + + def setUp(self): + """ + Set up module store testing capabilities and initialize test courses. + """ + super(CourseMetadataUtilsTestCase, self).setUp() + + mongo_builder = MongoModulestoreBuilder() + split_builder = VersioningModulestoreBuilder() + mixed_builder = MixedModulestoreBuilder([('mongo', mongo_builder), ('split', split_builder)]) + + with mixed_builder.build_without_contentstore() as (__, mixed_store): + with mixed_store.default_store('mongo'): + self.demo_course = mixed_store.create_course( + org="edX", + course="DemoX.1", + run="Fall_2014", + user_id=-3, # -3 refers to a "testing user" + fields={ + "start": _LAST_MONTH, + "end": _LAST_WEEK + } + ) + with mixed_store.default_store('split'): + self.html_course = mixed_store.create_course( + org="UniversityX", + course="CS-203", + run="Y2096", + user_id=-3, # -3 refers to a "testing user" + fields={ + "start": _NEXT_WEEK, + "display_name": "Intro to <html>" + } + ) + + def test_course_metadata_utils(self): + """ + Test every single function in course_metadata_utils. + """ + + def mock_strftime_localized(date_time, format_string): + """ + Mock version of strftime_localized used for testing purposes. + + Because we don't have a real implementation of strftime_localized + to work with (strftime_localized is provided by the XBlock runtime, + which we don't have access to for this test case), we must declare + this dummy implementation. This does NOT behave like a real + strftime_localized should. It purposely returns a really dumb value + that's only useful for testing purposes. + + Arguments: + date_time (datetime): datetime to be formatted. + format_string (str): format specifier. Valid values include: + - 'DATE_TIME' + - 'TIME' + - 'SHORT_DATE' + - 'LONG_DATE' + + Returns (str): format_string + " " + str(date_time) + """ + if format_string in ['DATE_TIME', 'TIME', 'SHORT_DATE', 'LONG_DATE']: + return format_string + " " + str(date_time) + else: + raise ValueError("Invalid format string :" + format_string) + + test_datetime = datetime(1945, 02, 06, 04, 20, 00, tzinfo=UTC()) + advertised_start_parsable = "2038-01-19 03:14:07" + advertised_start_unparsable = "This coming fall" + + FunctionTest = namedtuple('FunctionTest', 'function scenarios') # pylint: disable=invalid-name + TestScenario = namedtuple('TestScenario', 'arguments expected_return') # pylint: disable=invalid-name + + function_tests = [ + FunctionTest(clean_course_key, [ + TestScenario( + (self.demo_course.id, '='), + "course_MVSFQL2EMVWW6WBOGEXUMYLMNRPTEMBRGQ======" + ), + TestScenario( + (self.html_course.id, '~'), + "course_MNXXK4TTMUWXMMJ2KVXGS5TFOJZWS5DZLAVUGUZNGIYDGK2ZGIYDSNQ~" + ), + ]), + FunctionTest(url_name_for_course_location, [ + TestScenario((self.demo_course.location,), self.demo_course.location.name), + TestScenario((self.html_course.location,), self.html_course.location.name), + ]), + FunctionTest(display_name_with_default, [ + TestScenario((self.demo_course,), "Empty"), + TestScenario((self.html_course,), "Intro to <html>"), + ]), + FunctionTest(number_for_course_location, [ + TestScenario((self.demo_course.location,), "DemoX.1"), + TestScenario((self.html_course.location,), "CS-203"), + ]), + FunctionTest(has_course_started, [ + TestScenario((self.demo_course.start,), True), + TestScenario((self.html_course.start,), False), + ]), + FunctionTest(has_course_ended, [ + TestScenario((self.demo_course.end,), True), + TestScenario((self.html_course.end,), False), + ]), + FunctionTest(course_start_date_is_default, [ + TestScenario((test_datetime, advertised_start_parsable), False), + TestScenario((test_datetime, None), False), + TestScenario((DEFAULT_START_DATE, advertised_start_parsable), False), + TestScenario((DEFAULT_START_DATE, None), True), + ]), + FunctionTest(course_start_datetime_text, [ + TestScenario( + (DEFAULT_START_DATE, advertised_start_parsable, 'DATE_TIME', ugettext, mock_strftime_localized), + mock_strftime_localized(Date().from_json(advertised_start_parsable), 'DATE_TIME') + " UTC" + ), + TestScenario( + (test_datetime, advertised_start_unparsable, 'DATE_TIME', ugettext, mock_strftime_localized), + advertised_start_unparsable.title() + ), + TestScenario( + (test_datetime, None, 'SHORT_DATE', ugettext, mock_strftime_localized), + mock_strftime_localized(test_datetime, 'SHORT_DATE') + ), + TestScenario( + (DEFAULT_START_DATE, None, 'SHORT_DATE', ugettext, mock_strftime_localized), + # Translators: TBD stands for 'To Be Determined' and is used when a course + # does not yet have an announced start date. + ugettext('TBD') + ) + ]), + FunctionTest(course_end_datetime_text, [ + TestScenario( + (test_datetime, 'TIME', mock_strftime_localized), + mock_strftime_localized(test_datetime, 'TIME') + " UTC" + ), + TestScenario( + (None, 'TIME', mock_strftime_localized), + "" + ) + ]), + FunctionTest(may_certify_for_course, [ + TestScenario(('early_with_info', True, True), True), + TestScenario(('early_no_info', False, False), True), + TestScenario(('end', True, False), True), + TestScenario(('end', False, True), True), + TestScenario(('end', False, False), False), + ]), + ] + + for function_test in function_tests: + for scenario in function_test.scenarios: + actual_return = function_test.function(*scenario.arguments) + self.assertEqual(actual_return, scenario.expected_return) + + # Even though we don't care about testing mock_strftime_localized, + # we still need to test it with a bad format string in order to + # satisfy the coverage checker. + with self.assertRaises(ValueError): + mock_strftime_localized(test_datetime, 'BAD_FORMAT_SPECIFIER') diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index a9008d5511f584a41e696f296201f9581a889e7c..da05da360efb0c0b15d5db7db7b554710e3251a8 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -19,6 +19,10 @@ COURSE = 'test_course' NOW = datetime.strptime('2013-01-01T01:00:00', '%Y-%m-%dT%H:%M:00').replace(tzinfo=UTC()) +_TODAY = datetime.now(UTC()) +_LAST_WEEK = _TODAY - timedelta(days=7) +_NEXT_WEEK = _TODAY + timedelta(days=7) + class CourseFieldsTestCase(unittest.TestCase): def test_default_start_date(self): @@ -348,3 +352,49 @@ class TeamsConfigurationTestCase(unittest.TestCase): self.add_team_configuration(max_team_size=4, topics=topics) self.assertTrue(self.course.teams_enabled) self.assertEqual(self.course.teams_topics, topics) + + +class CourseDescriptorTestCase(unittest.TestCase): + """ + Tests for a select few functions from CourseDescriptor. + + I wrote these test functions in order to satisfy the coverage checker for + PR #8484, which modified some code within CourseDescriptor. However, this + class definitely isn't a comprehensive test case for CourseDescriptor, as + writing a such a test case was out of the scope of the PR. + """ + + def setUp(self): + """ + Initialize dummy testing course. + """ + super(CourseDescriptorTestCase, self).setUp() + self.course = get_dummy_course(start=_TODAY) + + def test_clean_id(self): + """ + Test CourseDescriptor.clean_id. + """ + self.assertEqual( + self.course.clean_id(), + "course_ORSXG5C7N5ZGOL3UMVZXIX3DN52XE43FF52GK43UL5ZHK3Q=" + ) + self.assertEqual( + self.course.clean_id(padding_char='$'), + "course_ORSXG5C7N5ZGOL3UMVZXIX3DN52XE43FF52GK43UL5ZHK3Q$" + ) + + def test_has_started(self): + """ + Test CourseDescriptor.has_started. + """ + self.course.start = _LAST_WEEK + self.assertTrue(self.course.has_started()) + self.course.start = _NEXT_WEEK + self.assertFalse(self.course.has_started()) + + def test_number(self): + """ + Test CourseDescriptor.number. + """ + self.assertEqual(self.course.number, COURSE) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index b7b5e27b048afafe2d9fc01ce0d551c41f4f25a6..9c809b1fad6a15e75273216e9d0766568dd96a76 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -26,10 +26,11 @@ from xblock.fields import ( ) from xblock.fragment import Fragment from xblock.runtime import Runtime, IdReader, IdGenerator +from xmodule import course_metadata_utils from xmodule.fields import RelativeTime - from xmodule.errortracker import exc_info_to_str from xmodule.modulestore.exceptions import ItemNotFoundError + from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.asides import AsideUsageKeyV1, AsideDefinitionKeyV1 from xmodule.exceptions import UndefinedContext @@ -335,7 +336,7 @@ class XModuleMixin(XModuleFields, XBlockMixin): @property def url_name(self): - return self.location.name + return course_metadata_utils.url_name_for_course_location(self.location) @property def display_name_with_default(self): @@ -343,10 +344,7 @@ class XModuleMixin(XModuleFields, XBlockMixin): Return a display name for the module: use display_name if defined in metadata, otherwise convert the url name. """ - name = self.display_name - if name is None: - name = self.url_name.replace('_', ' ') - return name.replace('<', '<').replace('>', '>') + return course_metadata_utils.display_name_with_default(self) @property def xblock_kvs(self): diff --git a/lms/envs/common.py b/lms/envs/common.py index 35432c036327de346e258f521f84b71db377cee5..1aa40c9cf5961c6677ab6ff84e9ee1f59db46149 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1889,6 +1889,7 @@ INSTALLED_APPS = ( 'lms.djangoapps.lms_xblock', + 'openedx.core.djangoapps.content.course_overviews', 'openedx.core.djangoapps.content.course_structures', 'course_structure_api', diff --git a/openedx/core/djangoapps/content/course_overviews/__init__.py b/openedx/core/djangoapps/content/course_overviews/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..c5bcc278aadfb7e036fea9038030743e39bb9a71 --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/__init__.py @@ -0,0 +1,28 @@ +""" +Library for quickly accessing basic course metadata. + +The rationale behind this app is that loading course metadata from the Split +Mongo Modulestore is too slow. See: + + https://openedx.atlassian.net/wiki/pages/viewpage.action?spaceKey=MA&title= + MA-296%3A+UserCourseEnrollmentList+Performance+Investigation + +This performance issue is not a problem when loading metadata for a *single* +course; however, there are many cases in LMS where we need to load metadata +for a number of courses simultaneously, which can cause very noticeable +latency. +Specifically, the endpoint /api/mobile_api/v0.5/users/{username}/course_enrollments +took an average of 900 ms, and all it does is generate a limited amount of data +for no more than a few dozen courses per user. + +In this app we declare the model CourseOverview, which caches course metadata +and a MySQL table and allows very quick access to it (according to NewRelic, +less than 1 ms). To load a CourseOverview, call CourseOverview.get_from_id +with the appropriate course key. The use cases for this app include things like +a user enrollment dashboard, a course metadata API, or a course marketing +page. +""" + +# importing signals is necessary to activate signal handler, which invalidates +# the CourseOverview cache every time a course is published +import openedx.core.djangoapps.content.course_overviews.signals # pylint: disable=unused-import diff --git a/openedx/core/djangoapps/content/course_overviews/migrations/0001_initial.py b/openedx/core/djangoapps/content/course_overviews/migrations/0001_initial.py new file mode 100644 index 0000000000000000000000000000000000000000..7c69d3cd9211e88e1d6042919fd4923c21a20057 --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/migrations/0001_initial.py @@ -0,0 +1,70 @@ +# -*- coding: utf-8 -*- +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'CourseOverview' + db.create_table('course_overviews_courseoverview', ( + ('id', self.gf('xmodule_django.models.CourseKeyField')(max_length=255, primary_key=True, db_index=True)), + ('_location', self.gf('xmodule_django.models.UsageKeyField')(max_length=255)), + ('display_name', self.gf('django.db.models.fields.TextField')(null=True)), + ('display_number_with_default', self.gf('django.db.models.fields.TextField')()), + ('display_org_with_default', self.gf('django.db.models.fields.TextField')()), + ('start', self.gf('django.db.models.fields.DateTimeField')(null=True)), + ('end', self.gf('django.db.models.fields.DateTimeField')(null=True)), + ('advertised_start', self.gf('django.db.models.fields.TextField')(null=True)), + ('course_image_url', self.gf('django.db.models.fields.TextField')()), + ('facebook_url', self.gf('django.db.models.fields.TextField')(null=True)), + ('social_sharing_url', self.gf('django.db.models.fields.TextField')(null=True)), + ('end_of_course_survey_url', self.gf('django.db.models.fields.TextField')(null=True)), + ('certificates_display_behavior', self.gf('django.db.models.fields.TextField')(null=True)), + ('certificates_show_before_end', self.gf('django.db.models.fields.BooleanField')(default=False)), + ('has_any_active_web_certificate', self.gf('django.db.models.fields.BooleanField')(default=False)), + ('cert_name_short', self.gf('django.db.models.fields.TextField')()), + ('cert_name_long', self.gf('django.db.models.fields.TextField')()), + ('lowest_passing_grade', self.gf('django.db.models.fields.DecimalField')(max_digits=5, decimal_places=2)), + ('mobile_available', self.gf('django.db.models.fields.BooleanField')(default=False)), + ('visible_to_staff_only', self.gf('django.db.models.fields.BooleanField')(default=False)), + ('_pre_requisite_courses_json', self.gf('django.db.models.fields.TextField')()), + )) + db.send_create_signal('course_overviews', ['CourseOverview']) + + + def backwards(self, orm): + # Deleting model 'CourseOverview' + db.delete_table('course_overviews_courseoverview') + + + models = { + 'course_overviews.courseoverview': { + 'Meta': {'object_name': 'CourseOverview'}, + '_location': ('xmodule_django.models.UsageKeyField', [], {'max_length': '255'}), + '_pre_requisite_courses_json': ('django.db.models.fields.TextField', [], {}), + 'advertised_start': ('django.db.models.fields.TextField', [], {'null': 'True'}), + 'cert_name_long': ('django.db.models.fields.TextField', [], {}), + 'cert_name_short': ('django.db.models.fields.TextField', [], {}), + 'certificates_display_behavior': ('django.db.models.fields.TextField', [], {'null': 'True'}), + 'certificates_show_before_end': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'course_image_url': ('django.db.models.fields.TextField', [], {}), + 'display_name': ('django.db.models.fields.TextField', [], {'null': 'True'}), + 'display_number_with_default': ('django.db.models.fields.TextField', [], {}), + 'display_org_with_default': ('django.db.models.fields.TextField', [], {}), + 'end': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}), + 'end_of_course_survey_url': ('django.db.models.fields.TextField', [], {'null': 'True'}), + 'facebook_url': ('django.db.models.fields.TextField', [], {'null': 'True'}), + 'has_any_active_web_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'primary_key': 'True', 'db_index': 'True'}), + 'lowest_passing_grade': ('django.db.models.fields.DecimalField', [], {'max_digits': '5', 'decimal_places': '2'}), + 'mobile_available': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'social_sharing_url': ('django.db.models.fields.TextField', [], {'null': 'True'}), + 'start': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}), + 'visible_to_staff_only': ('django.db.models.fields.BooleanField', [], {'default': 'False'}) + } + } + + complete_apps = ['course_overviews'] \ No newline at end of file diff --git a/openedx/core/djangoapps/content/course_overviews/migrations/__init__.py b/openedx/core/djangoapps/content/course_overviews/migrations/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py new file mode 100644 index 0000000000000000000000000000000000000000..b91e86f7620be40fc0b2c65f8e1a64dd916e7076 --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -0,0 +1,243 @@ +""" +Declaration of CourseOverview model +""" + +import json + +import django.db.models +from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField +from django.utils.translation import ugettext + +from lms.djangoapps.certificates.api import get_active_web_certificate +from lms.djangoapps.courseware.courses import course_image_url +from util.date_utils import strftime_localized +from xmodule import course_metadata_utils +from xmodule.modulestore.django import modulestore +from xmodule_django.models import CourseKeyField, UsageKeyField + + +class CourseOverview(django.db.models.Model): + """ + 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. + """ + + # Course identification + id = CourseKeyField(db_index=True, primary_key=True, max_length=255) # pylint: disable=invalid-name + _location = UsageKeyField(max_length=255) + display_name = TextField(null=True) + display_number_with_default = TextField() + display_org_with_default = TextField() + + # Start/end dates + start = DateTimeField(null=True) + end = DateTimeField(null=True) + advertised_start = TextField(null=True) + + # URLs + course_image_url = TextField() + facebook_url = TextField(null=True) + social_sharing_url = TextField(null=True) + end_of_course_survey_url = TextField(null=True) + + # Certification data + certificates_display_behavior = TextField(null=True) + certificates_show_before_end = BooleanField() + has_any_active_web_certificate = BooleanField() + cert_name_short = TextField() + cert_name_long = TextField() + + # Grading + lowest_passing_grade = DecimalField(max_digits=5, decimal_places=2) + + # Access parameters + mobile_available = BooleanField() + visible_to_staff_only = BooleanField() + _pre_requisite_courses_json = TextField() # JSON representation of list of CourseKey strings + + @staticmethod + def _create_from_course(course): + """ + Creates a CourseOverview object from a CourseDescriptor. + + Does not touch the database, simply constructs and returns an overview + from the given course. + + Arguments: + course (CourseDescriptor): any course descriptor object + + Returns: + CourseOverview: overview extracted from the given course + """ + return CourseOverview( + id=course.id, + _location=course.location, + display_name=course.display_name, + display_number_with_default=course.display_number_with_default, + display_org_with_default=course.display_org_with_default, + + start=course.start, + end=course.end, + advertised_start=course.advertised_start, + + course_image_url=course_image_url(course), + facebook_url=course.facebook_url, + social_sharing_url=course.social_sharing_url, + + certificates_display_behavior=course.certificates_display_behavior, + certificates_show_before_end=course.certificates_show_before_end, + has_any_active_web_certificate=(get_active_web_certificate(course) is not None), + cert_name_short=course.cert_name_short, + cert_name_long=course.cert_name_long, + lowest_passing_grade=course.lowest_passing_grade, + end_of_course_survey_url=course.end_of_course_survey_url, + + mobile_available=course.mobile_available, + visible_to_staff_only=course.visible_to_staff_only, + _pre_requisite_courses_json=json.dumps(course.pre_requisite_courses) + ) + + @staticmethod + def get_from_id(course_id): + """ + Load a CourseOverview object for a given course ID. + + First, we try to load the CourseOverview from the database. If it + doesn't exist, we load the entire course from the modulestore, create a + CourseOverview object from it, and then cache it in the database for + future use. + + Arguments: + course_id (CourseKey): the ID of the course overview to be loaded + + Returns: + CourseOverview: overview of the requested course + """ + course_overview = None + try: + course_overview = CourseOverview.objects.get(id=course_id) + except CourseOverview.DoesNotExist: + store = modulestore() + with store.bulk_operations(course_id): + course = store.get_course(course_id) + if course: + course_overview = CourseOverview._create_from_course(course) + course_overview.save() # Save new overview to the cache + return course_overview + + def clean_id(self, padding_char='='): + """ + Returns a unique deterministic base32-encoded ID for the course. + + Arguments: + padding_char (str): Character used for padding at end of base-32 + -encoded string, defaulting to '=' + """ + return course_metadata_utils.clean_course_key(self.location.course_key, padding_char) + + @property + def location(self): + """ + Returns the UsageKey of this course. + + UsageKeyField has a strange behavior where it fails to parse the "run" + of a course out of the serialized form of a Mongo Draft UsageKey. This + method is a wrapper around _location attribute that fixes the problem + by calling map_into_course, which restores the run attribute. + """ + if self._location.run is None: + self._location = self._location.map_into_course(self.id) + return self._location + + @property + def number(self): + """ + Returns this course's number. + + This is a "number" in the sense of the "course numbers" that you see at + lots of universities. For example, given a course + "Intro to Computer Science" with the course key "edX/CS-101/2014", the + course number would be "CS-101" + """ + return course_metadata_utils.number_for_course_location(self.location) + + @property + def url_name(self): + """ + Returns this course's URL name. + """ + return course_metadata_utils.url_name_for_course_location(self.location) + + @property + def display_name_with_default(self): + """ + Return reasonable display name for the course. + """ + return course_metadata_utils.display_name_with_default(self) + + def has_started(self): + """ + Returns whether the the course has started. + """ + return course_metadata_utils.has_course_started(self.start) + + def has_ended(self): + """ + Returns whether the course has ended. + """ + return course_metadata_utils.has_course_ended(self.end) + + def start_datetime_text(self, format_string="SHORT_DATE"): + """ + Returns the desired text corresponding the course's start date and + time in UTC. Prefers .advertised_start, then falls back to .start. + """ + return course_metadata_utils.course_start_datetime_text( + self.start, + self.advertised_start, + format_string, + ugettext, + strftime_localized + ) + + @property + def start_date_is_still_default(self): + """ + Checks if the start date set for the course is still default, i.e. + .start has not been modified, and .advertised_start has not been set. + """ + return course_metadata_utils.course_start_date_is_default( + self.start, + self.advertised_start, + ) + + def end_datetime_text(self, format_string="SHORT_DATE"): + """ + Returns the end date or datetime for the course formatted as a string. + """ + return course_metadata_utils.course_end_datetime_text( + self.end, + format_string, + strftime_localized + ) + + def may_certify(self): + """ + Returns whether it is acceptable to show the student a certificate + download link. + """ + return course_metadata_utils.may_certify_for_course( + self.certificates_display_behavior, + self.certificates_show_before_end, + self.has_ended() + ) + + @property + def pre_requisite_courses(self): + """ + Returns a list of ID strings for this course's prerequisite courses. + """ + return json.loads(self._pre_requisite_courses_json) diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py new file mode 100644 index 0000000000000000000000000000000000000000..37bfcd268155b996847119b8a4f0356cc8956d4a --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/signals.py @@ -0,0 +1,17 @@ +""" +Signal handler for invalidating cached course overviews +""" +from django.dispatch.dispatcher import receiver + +from xmodule.modulestore.django import SignalHandler + +from .models import CourseOverview + + +@receiver(SignalHandler.course_published) +def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument + """ + Catches the signal that a course has been published in Studio and + invalidates the corresponding CourseOverview cache entry if one exists. + """ + CourseOverview.objects.filter(id=course_key).delete() diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py new file mode 100644 index 0000000000000000000000000000000000000000..2ea41b2521ef2a075f23bf2d4e46408c8af39d83 --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/tests.py @@ -0,0 +1,258 @@ +""" +Tests for course_overviews app. +""" +import datetime +import ddt +import itertools +import pytz +import math + +from django.utils import timezone + +from lms.djangoapps.certificates.api import get_active_web_certificate +from lms.djangoapps.courseware.courses import course_image_url +from xmodule.course_metadata_utils import DEFAULT_START_DATE +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls, check_mongo_calls_range + +from .models import CourseOverview + + +@ddt.ddt +class CourseOverviewTestCase(ModuleStoreTestCase): + """ + Tests for CourseOverviewDescriptor model. + """ + + TODAY = timezone.now() + LAST_MONTH = TODAY - datetime.timedelta(days=30) + LAST_WEEK = TODAY - datetime.timedelta(days=7) + NEXT_WEEK = TODAY + datetime.timedelta(days=7) + NEXT_MONTH = TODAY + datetime.timedelta(days=30) + + def check_course_overview_against_course(self, course): + """ + Compares a CourseOverview object against its corresponding + CourseDescriptor object. + + Specifically, given a course, test that data within the following three + objects match each other: + - the CourseDescriptor itself + - a CourseOverview that was newly constructed from _create_from_course + - a CourseOverview that was loaded from the MySQL database + """ + + def get_seconds_since_epoch(date_time): + """ + Returns the number of seconds between the Unix Epoch and the given + datetime. If the given datetime is None, return None. + """ + if date_time is None: + return None + epoch = datetime.datetime.utcfromtimestamp(0).replace(tzinfo=pytz.utc) + return math.floor((date_time - epoch).total_seconds()) + + # Load the CourseOverview from the cache twice. The first load will be a cache miss (because the cache + # is empty) so the course will be newly created with CourseOverviewDescriptor.create_from_course. The second + # load will be a cache hit, so the course will be loaded from the cache. + course_overview_cache_miss = CourseOverview.get_from_id(course.id) + course_overview_cache_hit = CourseOverview.get_from_id(course.id) + + # Test if value of these attributes match between the three objects + fields_to_test = [ + 'id', + 'display_name', + 'display_number_with_default', + 'display_org_with_default', + 'advertised_start', + 'facebook_url', + 'social_sharing_url', + 'certificates_display_behavior', + 'certificates_show_before_end', + 'cert_name_short', + 'cert_name_long', + 'lowest_passing_grade', + 'end_of_course_survey_url', + 'mobile_available', + 'visible_to_staff_only', + 'location', + 'number', + 'url_name', + 'display_name_with_default', + 'start_date_is_still_default', + 'pre_requisite_courses', + ] + for attribute_name in fields_to_test: + course_value = getattr(course, attribute_name) + cache_miss_value = getattr(course_overview_cache_miss, attribute_name) + cache_hit_value = getattr(course_overview_cache_hit, attribute_name) + self.assertEqual(course_value, cache_miss_value) + self.assertEqual(cache_miss_value, cache_hit_value) + + # Test if return values for all methods are equal between the three objects + methods_to_test = [ + ('clean_id', ()), + ('clean_id', ('#',)), + ('has_ended', ()), + ('has_started', ()), + ('start_datetime_text', ('SHORT_DATE',)), + ('start_datetime_text', ('DATE_TIME',)), + ('end_datetime_text', ('SHORT_DATE',)), + ('end_datetime_text', ('DATE_TIME',)), + ('may_certify', ()), + ] + for method_name, method_args in methods_to_test: + course_value = getattr(course, method_name)(*method_args) + cache_miss_value = getattr(course_overview_cache_miss, method_name)(*method_args) + cache_hit_value = getattr(course_overview_cache_hit, method_name)(*method_args) + self.assertEqual(course_value, cache_miss_value) + 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 + # 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. + others_to_test = [( + 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), + )] + for (course_value, cache_miss_value, cache_hit_value) in others_to_test: + self.assertEqual(course_value, cache_miss_value) + self.assertEqual(cache_miss_value, cache_hit_value) + + @ddt.data(*itertools.product( + [ + { + "display_name": "Test Course", # Display name provided + "start": LAST_WEEK, # In the middle of the course + "end": NEXT_WEEK, + "advertised_start": "2015-01-01 11:22:33", # Parse-able advertised_start + "pre_requisite_courses": [ # Has pre-requisites + 'course-v1://edX+test1+run1', + 'course-v1://edX+test2+run1' + ], + "static_asset_path": "/my/abs/path", # Absolute path + "certificates_show_before_end": True, + }, + { + "display_name": "", # Empty display name + "start": NEXT_WEEK, # Course hasn't started yet + "end": NEXT_MONTH, + "advertised_start": "Very Soon!", # Not parse-able advertised_start + "pre_requisite_courses": [], # No pre-requisites + "static_asset_path": "my/relative/path", # Relative asset path + "certificates_show_before_end": False, + }, + { + "display_name": "", # Empty display name + "start": LAST_MONTH, # Course already ended + "end": LAST_WEEK, + "advertised_start": None, # No advertised start + "pre_requisite_courses": [], # No pre-requisites + "static_asset_path": "", # Empty asset path + "certificates_show_before_end": False, + }, + { + # # Don't set display name + "start": DEFAULT_START_DATE, # Default start and end dates + "end": None, + "advertised_start": None, # No advertised start + "pre_requisite_courses": [], # No pre-requisites + "static_asset_path": None, # No asset path + "certificates_show_before_end": False, + } + ], + [ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split] + )) + @ddt.unpack + def test_course_overview_behavior(self, course_kwargs, modulestore_type): + """ + Tests if CourseOverviews and CourseDescriptors behave the same + by comparing pairs of them given a variety of scenarios. + + Arguments: + course_kwargs (dict): kwargs to be passed to course constructor + modulestore_type (ModuleStoreEnum.Type) + is_user_enrolled (bool) + """ + + course = CourseFactory.create( + course="TEST101", + org="edX", + run="Run1", + default_store=modulestore_type, + **course_kwargs + ) + self.check_course_overview_against_course(course) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_course_overview_cache_invalidation(self, modulestore_type): + """ + Tests that when a course is published, the corresponding + course_overview is removed from the cache. + """ + with self.store.default_store(modulestore_type): + + # Create a course where mobile_available is True. + course = CourseFactory.create( + course="TEST101", + org="edX", + run="Run1", + mobile_available=True, + default_store=modulestore_type + ) + course_overview_1 = CourseOverview.get_from_id(course.id) + self.assertTrue(course_overview_1.mobile_available) + + # Set mobile_available to False and update the course. + # This fires a course_published signal, which should be caught in signals.py, which should in turn + # delete the corresponding CourseOverview from the cache. + course.mobile_available = False + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.store.update_item(course, ModuleStoreEnum.UserID.test) + + # Make sure that when we load the CourseOverview again, mobile_available is updated. + course_overview_2 = CourseOverview.get_from_id(course.id) + self.assertFalse(course_overview_2.mobile_available) + + @ddt.data((ModuleStoreEnum.Type.mongo, 1, 1), (ModuleStoreEnum.Type.split, 3, 4)) + @ddt.unpack + def test_course_overview_caching(self, modulestore_type, min_mongo_calls, max_mongo_calls): + """ + Tests that CourseOverview structures are actually getting cached. + """ + course = CourseFactory.create( + course="TEST101", + org="edX", + run="Run1", + mobile_available=True, + default_store=modulestore_type + ) + + # The first time we load a CourseOverview, it will be a cache miss, so + # we expect the modulestore to be queried. + with check_mongo_calls_range(max_finds=max_mongo_calls, min_finds=min_mongo_calls): + _course_overview_1 = CourseOverview.get_from_id(course.id) + + # The second time we load a CourseOverview, it will be a cache hit, so + # we expect no modulestore queries to be made. + with check_mongo_calls(0): + _course_overview_2 = CourseOverview.get_from_id(course.id)