Skip to content

E
edx-platform
Commit c14c146d by Nimisha Asthagiri
Options

Merge pull request #10841 from edx/mobile/optimize-course-api 

Optimize Course Catalog using CourseOverview
parents d54ac1eb 2b8441a0
Showing with 630 additions and 453 deletions
common/djangoapps/enrollment/tests/test_views.py
......@@ -150,10 +150,9 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase):
throttle = EnrollmentUserThrottle()
self.rate_limit, rate_duration = throttle.parse_rate(throttle.rate)
self.course = CourseFactory.create()
# Load a CourseOverview. This initial load should result in a cache
# miss; the modulestore is queried and course metadata is cached.
__ = CourseOverview.get_from_id(self.course.id)
# Pass emit_signals when creating the course so it would be cached
# as a CourseOverview.
self.course = CourseFactory.create(emit_signals=True)
self.user = UserFactory.create(
username=self.USERNAME,
......@@ -336,7 +335,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase):
requesting user.
"""
# Create another course, and enroll self.user in both courses.
other_course = CourseFactory.create()
other_course = CourseFactory.create(emit_signals=True)
for course in self.course, other_course:
CourseModeFactory.create(
course_id=unicode(course.id),
......@@ -345,7 +344,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase):
)
self.assert_enrollment_status(
course_id=unicode(course.id),
max_mongo_calls=1,
max_mongo_calls=0,
)
# Verify the user himself can see both of his enrollments.
self._assert_enrollments_visible_in_list([self.course, other_course])
......
common/lib/xmodule/xmodule/course_metadata_utils.py
......@@ -5,9 +5,10 @@ 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 datetime import timedelta
from base64 import b32encode
from datetime import datetime, timedelta
import dateutil.parser
from math import exp
from django.utils.timezone import UTC
......@@ -222,3 +223,43 @@ def may_certify_for_course(certificates_display_behavior, certificates_show_befo
or certificates_show_before_end
)
return show_early or has_ended
def sorting_score(start, advertised_start, announcement):
"""
Returns a tuple that can be used to sort the courses according
to 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.
"""
# Make courses that have an announcement date have a lower
# score than courses than don't, older courses should have a
# higher score.
announcement, start, now = sorting_dates(start, advertised_start, announcement)
scale = 300.0 # about a year
if announcement:
days = (now - announcement).days
score = -exp(-days / scale)
else:
days = (now - start).days
score = exp(days / scale)
return score
def sorting_dates(start, advertised_start, announcement):
"""
Utility function to get datetime objects for dates used to
compute the is_new flag and the sorting_score.
"""
try:
start = dateutil.parser.parse(advertised_start)
if start.tzinfo is None:
start = start.replace(tzinfo=UTC())
except (ValueError, AttributeError):
start = start
now = datetime.now(UTC())
return announcement, start, now
common/lib/xmodule/xmodule/course_module.py
......@@ -3,12 +3,10 @@ Django module container for classes and operations related to the "Course Module
"""
import logging
from cStringIO import StringIO
from math import exp
from lxml import etree
from path import Path as path
import requests
from datetime import datetime
import dateutil.parser
from lazy import lazy
from xmodule import course_metadata_utils
......@@ -1264,7 +1262,9 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin):
flag = self.is_new
if flag is None:
# Use a heuristic if the course has not been flagged
announcement, start, now = self._sorting_dates()
announcement, start, now = course_metadata_utils.sorting_dates(
self.start, self.advertised_start, self.announcement
)
if announcement and (now - announcement).days < 30:
# The course has been announced for less that month
return True
......@@ -1284,41 +1284,11 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin):
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
(advertized) start dates of the course if available.
(advertised) start dates of the course if available.
The lower the number the "newer" the course.
"""
# Make courses that have an announcement date shave a lower
# score than courses than don't, older courses should have a
# higher score.
announcement, start, now = self._sorting_dates()
scale = 300.0 # about a year
if announcement:
days = (now - announcement).days
score = -exp(-days / scale)
else:
days = (now - start).days
score = exp(days / scale)
return score
def _sorting_dates(self):
# utility function to get datetime objects for dates used to
# compute the is_new flag and the sorting_score
announcement = self.announcement
if announcement is not None:
announcement = announcement
try:
start = dateutil.parser.parse(self.advertised_start)
if start.tzinfo is None:
start = start.replace(tzinfo=UTC())
except (ValueError, AttributeError):
start = self.start
now = datetime.now(UTC())
return announcement, start, now
return course_metadata_utils.sorting_score(self.start, self.advertised_start, self.announcement)
@lazy
def grading_context(self):
......
common/lib/xmodule/xmodule/tests/test_course_module.py
......@@ -150,14 +150,14 @@ class IsNewCourseTestCase(unittest.TestCase):
# Needed for test_is_newish
datetime_patcher = patch.object(
xmodule.course_module, 'datetime',
xmodule.course_metadata_utils, 'datetime',
Mock(wraps=datetime)
)
mocked_datetime = datetime_patcher.start()
mocked_datetime.now.return_value = NOW
self.addCleanup(datetime_patcher.stop)
@patch('xmodule.course_module.datetime.now')
@patch('xmodule.course_metadata_utils.datetime.now')
def test_sorting_score(self, gmtime_mock):
gmtime_mock.return_value = NOW
......@@ -208,7 +208,7 @@ class IsNewCourseTestCase(unittest.TestCase):
(xmodule.course_module.CourseFields.start.default, 'January 2014', 'January 2014', False, 'January 2014'),
]
@patch('xmodule.course_module.datetime.now')
@patch('xmodule.course_metadata_utils.datetime.now')
def test_start_date_text(self, gmtime_mock):
gmtime_mock.return_value = NOW
for s in self.start_advertised_settings:
......@@ -216,7 +216,7 @@ class IsNewCourseTestCase(unittest.TestCase):
print "Checking start=%s advertised=%s" % (s[0], s[1])
self.assertEqual(d.start_datetime_text(), s[2])
@patch('xmodule.course_module.datetime.now')
@patch('xmodule.course_metadata_utils.datetime.now')
def test_start_date_time_text(self, gmtime_mock):
gmtime_mock.return_value = NOW
for setting in self.start_advertised_settings:
......
common/test/acceptance/pages/studio/settings_advanced.py
......@@ -178,7 +178,6 @@ class AdvancedSettingsPage(CoursePage):
'display_name',
'info_sidebar_name',
'is_new',
'ispublic',
'issue_badges',
'max_student_enrollments_allowed',
'no_grade',
......
lms/djangoapps/branding/__init__.py
......@@ -14,27 +14,22 @@ from django.conf import settings
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from microsite_configuration import microsite
from django.contrib.staticfiles.storage import staticfiles_storage
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
def get_visible_courses():
"""
Return the set of CourseDescriptors that should be visible in this branded instance
"""
filtered_by_org = microsite.get_value('course_org_filter')
_courses = modulestore().get_courses(org=filtered_by_org)
courses = [c for c in _courses
if isinstance(c, CourseDescriptor)]
courses = CourseOverview.get_all_courses(org=filtered_by_org)
courses = sorted(courses, key=lambda course: course.number)
subdomain = microsite.get_value('subdomain', 'default')
# See if we have filtered course listings in this domain
filtered_visible_ids = None
# this is legacy format which is outside of the microsite feature -- also handle dev case, which should not filter
subdomain = microsite.get_value('subdomain', 'default')
if hasattr(settings, 'COURSE_LISTINGS') and subdomain in settings.COURSE_LISTINGS and not settings.DEBUG:
filtered_visible_ids = frozenset(
[SlashSeparatedCourseKey.from_deprecated_string(c) for c in settings.COURSE_LISTINGS[subdomain]]
......
lms/djangoapps/class_dashboard/views.py
......@@ -8,7 +8,7 @@ import json
from django.http import HttpResponse
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from courseware.courses import get_course_with_access
from courseware.courses import get_course_overview_with_access
from courseware.access import has_access
from class_dashboard import dashboard_data
......@@ -21,7 +21,7 @@ def has_instructor_access_for_class(user, course_id):
Returns true if the `user` is an instructor for the course.
"""
course = get_course_with_access(user, 'staff', course_id, depth=None)
course = get_course_overview_with_access(user, 'staff', course_id)
return bool(has_access(user, 'staff', course))
......
lms/djangoapps/course_api/api.py
......@@ -3,10 +3,13 @@ Course API
"""
from django.contrib.auth.models import User
from django.http import Http404
from rest_framework.exceptions import NotFound, PermissionDenied
from rest_framework.exceptions import PermissionDenied
from lms.djangoapps.courseware.courses import get_courses, get_course_with_access
from lms.djangoapps.courseware.courses import (
get_courses,
get_course_overview_with_access,
get_permission_for_course_about,
)
from .permissions import can_view_courses_for_username
......@@ -43,11 +46,11 @@ def course_detail(request, username, course_key):
`CourseDescriptor` object representing the requested course
"""
user = get_effective_user(request.user, username)
try:
course = get_course_with_access(user, 'see_exists', course_key)
except Http404:
raise NotFound()
return course
return get_course_overview_with_access(
user,
get_permission_for_course_about(),
course_key,
)
def list_courses(request, username):
......@@ -71,5 +74,4 @@ def list_courses(request, username):
List of `CourseDescriptor` objects representing the collection of courses.
"""
user = get_effective_user(request.user, username)
courses = get_courses(user)
return courses
return get_courses(user)
lms/djangoapps/course_api/blocks/views.py
......@@ -39,8 +39,6 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
* username: (string) The name of the user on whose behalf we want to
see the data.
Default is the logged in user
Example: username=anjali
* student_view_data: (list) Indicates for which block types to return
......
lms/djangoapps/course_api/serializers.py
......@@ -5,42 +5,32 @@ Course API Serializers. Representing course catalog data
import urllib
from django.core.urlresolvers import reverse
from django.template import defaultfilters
from rest_framework import serializers
from lms.djangoapps.courseware.courses import get_course_about_section
from openedx.core.lib.courses import course_image_url
from openedx.core.djangoapps.models.course_details import CourseDetails
from xmodule.course_module import DEFAULT_START_DATE
class _MediaSerializer(serializers.Serializer): # pylint: disable=abstract-method
"""
Nested serializer to represent a media object.
"""
def __init__(self, uri_parser, *args, **kwargs):
def __init__(self, uri_attribute, *args, **kwargs):
super(_MediaSerializer, self).__init__(*args, **kwargs)
self.uri_parser = uri_parser
self.uri_attribute = uri_attribute
uri = serializers.SerializerMethodField(source='*')
def get_uri(self, course):
def get_uri(self, course_overview):
"""
Get the representation for the media resource's URI
"""
return self.uri_parser(course)
return getattr(course_overview, self.uri_attribute)
class _CourseApiMediaCollectionSerializer(serializers.Serializer): # pylint: disable=abstract-method
"""
Nested serializer to represent a collection of media objects
"""
course_image = _MediaSerializer(source='*', uri_parser=course_image_url)
course_video = _MediaSerializer(
source='*',
uri_parser=lambda course: CourseDetails.fetch_video_url(course.id),
)
course_image = _MediaSerializer(source='*', uri_attribute='course_image_url')
course_video = _MediaSerializer(source='*', uri_attribute='course_video_url')
class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-method
......@@ -52,14 +42,14 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth
name = serializers.CharField(source='display_name_with_default')
number = serializers.CharField(source='display_number_with_default')
org = serializers.CharField(source='display_org_with_default')
short_description = serializers.SerializerMethodField()
effort = serializers.SerializerMethodField()
short_description = serializers.CharField()
effort = serializers.CharField()
media = _CourseApiMediaCollectionSerializer(source='*')
start = serializers.DateTimeField()
start_type = serializers.SerializerMethodField()
start_display = serializers.SerializerMethodField()
start_type = serializers.CharField()
start_display = serializers.CharField()
end = serializers.DateTimeField()
enrollment_start = serializers.DateTimeField()
......@@ -67,46 +57,12 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth
blocks_url = serializers.SerializerMethodField()
def get_start_type(self, course):
"""
Get the representation for SerializerMethodField `start_type`
"""
if course.advertised_start is not None:
return u'string'
elif course.start != DEFAULT_START_DATE:
return u'timestamp'
else:
return u'empty'
def get_start_display(self, course):
"""
Get the representation for SerializerMethodField `start_display`
"""
if course.advertised_start is not None:
return course.advertised_start
elif course.start != DEFAULT_START_DATE:
return defaultfilters.date(course.start, "DATE_FORMAT")
else:
return None
def get_short_description(self, course):
"""
Get the representation for SerializerMethodField `short_description`
"""
return get_course_about_section(self.context['request'], course, 'short_description').strip()
def get_blocks_url(self, course):
def get_blocks_url(self, course_overview):
"""
Get the representation for SerializerMethodField `blocks_url`
"""
base_url = '?'.join([
reverse('blocks_in_course'),
urllib.urlencode({'course_id': course.id}),
urllib.urlencode({'course_id': course_overview.id}),
])
return self.context['request'].build_absolute_uri(base_url)
def get_effort(self, course):
"""
Get the representation for SerializerMethodField `effort`
"""
return CourseDetails.fetch_effort(course.id)
lms/djangoapps/course_api/tests/mixins.py
......@@ -26,6 +26,7 @@ class CourseApiFactoryMixin(object):
end=datetime(2015, 9, 19, 18, 0, 0),
enrollment_start=datetime(2015, 6, 15, 0, 0, 0),
enrollment_end=datetime(2015, 7, 15, 0, 0, 0),
emit_signals=True,
**kwargs
)
......
lms/djangoapps/course_api/tests/test_api.py
......@@ -3,13 +3,15 @@ Test for course API
"""
from django.contrib.auth.models import AnonymousUser
from rest_framework.exceptions import NotFound, PermissionDenied
from django.http import Http404
from rest_framework.exceptions import PermissionDenied
from rest_framework.request import Request
from rest_framework.test import APIRequestFactory
from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, ModuleStoreTestCase
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.tests.factories import check_mongo_calls
from ..api import course_detail, list_courses
from .mixins import CourseApiFactoryMixin
......@@ -23,12 +25,12 @@ class CourseApiTestMixin(CourseApiFactoryMixin):
def setUpClass(cls):
super(CourseApiTestMixin, cls).setUpClass()
cls.request_factory = APIRequestFactory()
CourseOverview.get_all_courses() # seed the CourseOverview table
def verify_course(self, course, course_id=u'edX/toy/2012_Fall'):
"""
Ensure that the returned course is the course we just created
"""
self.assertIsInstance(course, CourseDescriptor)
self.assertEqual(course_id, str(course.id))
......@@ -43,6 +45,7 @@ class CourseDetailTestMixin(CourseApiTestMixin):
"""
request = Request(self.request_factory.get('/'))
request.user = requesting_user
with check_mongo_calls(0):
return course_detail(request, target_user.username, course_key)
......@@ -64,11 +67,11 @@ class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
def test_get_nonexistent_course(self):
course_key = CourseKey.from_string(u'edX/toy/nope')
with self.assertRaises(NotFound):
with self.assertRaises(Http404):
self._make_api_call(self.honor_user, self.honor_user, course_key)
def test_hidden_course_for_honor(self):
with self.assertRaises(NotFound):
with self.assertRaises(Http404):
self._make_api_call(self.honor_user, self.honor_user, self.hidden_course.id)
def test_hidden_course_for_staff(self):
......@@ -76,7 +79,7 @@ class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
self.verify_course(course, course_id=u'edX/hidden/2012_Fall')
def test_hidden_course_for_staff_as_honor(self):
with self.assertRaises(NotFound):
with self.assertRaises(Http404):
self._make_api_call(self.staff_user, self.honor_user, self.hidden_course.id)
......@@ -91,6 +94,7 @@ class CourseListTestMixin(CourseApiTestMixin):
"""
request = Request(self.request_factory.get('/'))
request.user = requesting_user
with check_mongo_calls(0):
return list_courses(request, specified_user.username)
def verify_courses(self, courses):
......
lms/djangoapps/course_api/tests/test_serializers.py
......@@ -5,6 +5,7 @@ Test data created by CourseSerializer
from datetime import datetime
from openedx.core.djangoapps.models.course_details import CourseDetails
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from rest_framework.test import APIRequestFactory
from rest_framework.request import Request
......@@ -29,7 +30,7 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
def _get_request(self, user=None):
"""
Build a Request object for the specified user
Build a Request object for the specified user.
"""
if user is None:
user = self.honor_user
......@@ -37,6 +38,13 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
request.user = user
return request
def _get_result(self, course):
"""
Return the CourseSerializer for the specified course.
"""
course_overview = CourseOverview.get_from_id(course.id)
return CourseSerializer(course_overview, context={'request': self._get_request()}).data
def test_basic(self):
expected_data = {
'course_id': u'edX/toy/2012_Fall',
......@@ -55,15 +63,15 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
'start': u'2015-07-17T12:00:00Z',
'start_type': u'timestamp',
'start_display': u'July 17, 2015',
'end': u'2015-09-19T18:00:00',
'enrollment_start': u'2015-06-15T00:00:00',
'enrollment_end': u'2015-07-15T00:00:00',
'end': u'2015-09-19T18:00:00Z',
'enrollment_start': u'2015-06-15T00:00:00Z',
'enrollment_end': u'2015-07-15T00:00:00Z',
'blocks_url': u'http://testserver/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall',
'effort': u'6 hours',
}
course = self.create_course()
CourseDetails.update_about_video(course, 'test_youtube_id', self.staff_user.id) # pylint: disable=no-member
result = CourseSerializer(course, context={'request': self._get_request()}).data
result = self._get_result(course)
self.assertDictEqual(result, expected_data)
def test_advertised_start(self):
......@@ -72,14 +80,14 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
start=datetime(2015, 3, 15),
advertised_start=u'The Ides of March'
)
result = CourseSerializer(course, context={'request': self._get_request()}).data
result = self._get_result(course)
self.assertEqual(result['course_id'], u'edX/custom/2012_Fall')
self.assertEqual(result['start_type'], u'string')
self.assertEqual(result['start_display'], u'The Ides of March')
def test_empty_start(self):
course = self.create_course(start=DEFAULT_START_DATE, course=u'custom')
result = CourseSerializer(course, context={'request': self._get_request()}).data
result = self._get_result(course)
self.assertEqual(result['course_id'], u'edX/custom/2012_Fall')
self.assertEqual(result['start_type'], u'empty')
self.assertIsNone(result['start_display'])
lms/djangoapps/course_api/views.py
......@@ -2,7 +2,7 @@
Course API Views
"""
from rest_framework.exceptions import NotFound
from django.http import Http404
from rest_framework.generics import ListAPIView, RetrieveAPIView
from opaque_keys import InvalidKeyError
......@@ -102,7 +102,7 @@ class CourseDetailView(RetrieveAPIView):
try:
course_key = CourseKey.from_string(course_key_string)
except InvalidKeyError:
raise NotFound()
raise Http404()
return course_detail(self.request, username, course_key)
......
lms/djangoapps/course_wiki/middleware.py
......@@ -6,7 +6,7 @@ from django.shortcuts import redirect
from django.core.exceptions import PermissionDenied
from wiki.models import reverse
from courseware.courses import get_course_with_access
from courseware.courses import get_course_with_access, get_course_overview_with_access
from courseware.access import has_access
from student.models import CourseEnrollment
from util.request import course_id_from_url
......@@ -29,7 +29,7 @@ class WikiAccessMiddleware(object):
if course_id:
# See if we are able to view the course. If we are, redirect to it
try:
_course = get_course_with_access(request.user, 'load', course_id)
get_course_overview_with_access(request.user, 'load', course_id)
return redirect("/courses/{course_id}/wiki/{path}".format(course_id=course_id.to_deprecated_string(), path=wiki_path))
except Http404:
# Even though we came from the course, we can't see it. So don't worry about it.
......
lms/djangoapps/courseware/access.py
......@@ -105,10 +105,10 @@ def has_access(user, action, obj, course_key=None):
# delegate the work to type-specific functions.
# (start with more specific types, then get more general)
if isinstance(obj, CourseDescriptor):
return _has_access_course_desc(user, action, obj)
return _has_access_course(user, action, obj)
if isinstance(obj, CourseOverview):
return _has_access_course_overview(user, action, obj)
return _has_access_course(user, action, obj)
if isinstance(obj, ErrorDescriptor):
return _has_access_error_desc(user, action, obj, course_key)
......@@ -270,17 +270,22 @@ def _can_enroll_courselike(user, courselike):
return ACCESS_DENIED
def _has_access_course_desc(user, action, course):
def _has_access_course(user, action, courselike):
"""
Check if user has access to a course descriptor.
Check if user has access to a course.
Arguments:
user (User): the user whose course access we are checking.
action (string): The action that is being checked.
courselike (CourseDescriptor or CourseOverview): The object
representing the course that the user wants to access.
Valid actions:
'load' -- load the courseware, see inside the course
'load_forum' -- can load and contribute to the forums (one access level for now)
'load_mobile' -- can load from a mobile context
'enroll' -- enroll. Checks for enrollment window,
ACCESS_REQUIRE_STAFF_FOR_COURSE,
'enroll' -- enroll. Checks for enrollment window.
'see_exists' -- can see that the course exists.
'staff' -- staff access to course.
'see_in_catalog' -- user is able to see the course listed in the course catalog.
......@@ -292,36 +297,27 @@ def _has_access_course_desc(user, action, course):
NOTE: this is not checking whether user is actually enrolled in the course.
"""
# delegate to generic descriptor check to check start dates
return _has_access_descriptor(user, 'load', course, course.id)
response = (
_visible_to_nonstaff_users(courselike) and
_can_access_descriptor_with_start_date(user, courselike, courselike.id)
)
return (
ACCESS_GRANTED if (response or _has_staff_access_to_descriptor(user, courselike, courselike.id))
else response
)
def can_enroll():
return _can_enroll_courselike(user, course)
"""
Returns whether the user can enroll in the course.
"""
return _can_enroll_courselike(user, courselike)
def see_exists():
"""
Can see if can enroll, but also if can load it: if user enrolled in a course and now
it's past the enrollment period, they should still see it.
"""
# VS[compat] -- this setting should go away once all courses have
# properly configured enrollment_start times (if course should be
# staff-only, set enrollment_start far in the future.)
if settings.FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'):
dog_stats_api.increment(
DEPRECATION_VSCOMPAT_EVENT,
tags=(
"location:has_access_course_desc_see_exists",
u"course:{}".format(course),
)
)
# if this feature is on, only allow courses that have ispublic set to be
# seen by non-staff
if course.ispublic:
debug("Allow: ACCESS_REQUIRE_STAFF_FOR_COURSE and ispublic")
return ACCESS_GRANTED
return _has_staff_access_to_descriptor(user, course, course.id)
return ACCESS_GRANTED if (can_enroll() or can_load()) else ACCESS_DENIED
def can_see_in_catalog():
......@@ -331,8 +327,8 @@ def _has_access_course_desc(user, action, course):
but also allow course staff to see this.
"""
return (
_has_catalog_visibility(course, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
or _has_staff_access_to_descriptor(user, course, course.id)
_has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
or _has_staff_access_to_descriptor(user, courselike, courselike.id)
)
def can_see_about_page():
......@@ -342,75 +338,25 @@ def _has_access_course_desc(user, action, course):
but also allow course staff to see this.
"""
return (
_has_catalog_visibility(course, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
or _has_catalog_visibility(course, CATALOG_VISIBILITY_ABOUT)
or _has_staff_access_to_descriptor(user, course, course.id)
_has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
or _has_catalog_visibility(courselike, CATALOG_VISIBILITY_ABOUT)
or _has_staff_access_to_descriptor(user, courselike, courselike.id)
)
checkers = {
'load': can_load,
'view_courseware_with_prerequisites':
lambda: _can_view_courseware_with_prerequisites(user, course),
'load_mobile': lambda: can_load() and _can_load_course_on_mobile(user, course),
lambda: _can_view_courseware_with_prerequisites(user, courselike),
'load_mobile': lambda: can_load() and _can_load_course_on_mobile(user, courselike),
'enroll': can_enroll,
'see_exists': see_exists,
'staff': lambda: _has_staff_access_to_descriptor(user, course, course.id),
'instructor': lambda: _has_instructor_access_to_descriptor(user, course, course.id),
'staff': lambda: _has_staff_access_to_descriptor(user, courselike, courselike.id),
'instructor': lambda: _has_instructor_access_to_descriptor(user, courselike, courselike.id),
'see_in_catalog': can_see_in_catalog,
'see_about_page': can_see_about_page,
}
return _dispatch(checkers, action, user, course)
def _can_load_course_overview(user, course_overview):
"""
Check if a user can load a course overview.
Arguments:
user (User): the user whose course access we are checking.
course_overview (CourseOverview): a course overview.
Note:
The user doesn't have to be enrolled in the course in order to have load
load access.
"""
response = (
_visible_to_nonstaff_users(course_overview)
and _can_access_descriptor_with_start_date(user, course_overview, course_overview.id)
)
return (
ACCESS_GRANTED if (response or _has_staff_access_to_descriptor(user, course_overview, course_overview.id))
else response
)
_COURSE_OVERVIEW_CHECKERS = {
'enroll': _can_enroll_courselike,
'load': _can_load_course_overview,
'load_mobile': lambda user, course_overview: (
_can_load_course_overview(user, course_overview)
and _can_load_course_on_mobile(user, course_overview)
),
'view_courseware_with_prerequisites': _can_view_courseware_with_prerequisites
}
COURSE_OVERVIEW_SUPPORTED_ACTIONS = _COURSE_OVERVIEW_CHECKERS.keys()
def _has_access_course_overview(user, action, course_overview):
"""
Check if user has access to a course overview.
Arguments:
user (User): the user whose course access we are checking.
action (str): the action the user is trying to perform.
See COURSE_OVERVIEW_SUPPORTED_ACTIONS for valid values.
course_overview (CourseOverview): overview of the course in question.
"""
if action in _COURSE_OVERVIEW_CHECKERS:
return _COURSE_OVERVIEW_CHECKERS[action](user, course_overview)
else:
raise ValueError(u"Unknown action for object type 'CourseOverview': '{}'".format(action))
return _dispatch(checkers, action, user, courselike)
def _has_access_error_desc(user, action, descriptor, course_key):
......
lms/djangoapps/courseware/courses.py
......@@ -14,7 +14,6 @@ from django.conf import settings
from edxmako.shortcuts import render_to_string
from xmodule.modulestore import ModuleStoreEnum
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from static_replace import replace_static_urls
......@@ -37,6 +36,7 @@ from student.models import CourseEnrollment
import branding
from opaque_keys.edx.keys import UsageKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
log = logging.getLogger(__name__)
......@@ -58,7 +58,6 @@ def get_course(course_id, depth=0):
return course
# TODO please rename this function to get_course_by_key at next opportunity!
def get_course_by_id(course_key, depth=0):
"""
Given a course id, return the corresponding course descriptor.
......@@ -94,9 +93,39 @@ def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled=
check_if_enrolled: If true, additionally verifies that the user is either enrolled in the course
or has staff access.
"""
assert isinstance(course_key, CourseKey)
course = get_course_by_id(course_key, depth=depth)
access_response = has_access(user, action, course, course_key)
course = get_course_by_id(course_key, depth)
check_course_access(course, user, action, check_if_enrolled)
return course
def get_course_overview_with_access(user, action, course_key, check_if_enrolled=False):
"""
Given a course_key, look up the corresponding course overview,
check that the user has the access to perform the specified action
on the course, and return the course overview.
Raises a 404 if the course_key is invalid, or the user doesn't have access.
check_if_enrolled: If true, additionally verifies that the user is either enrolled in the course
or has staff access.
"""
try:
course_overview = CourseOverview.get_from_id(course_key)
except CourseOverview.DoesNotExist:
raise Http404("Course not found.")
check_course_access(course_overview, user, action, check_if_enrolled)
return course_overview
def check_course_access(course, user, action, check_if_enrolled=False):
"""
Check that the user has the access to perform the specified action
on the course (CourseDescriptor|CourseOverview).
check_if_enrolled: If true, additionally verifies that the user is either
enrolled in the course or has staff access.
"""
access_response = has_access(user, action, course, course.id)
if not access_response:
# Deliberately return a non-specific error message to avoid
......@@ -104,12 +133,11 @@ def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled=
raise CoursewareAccessException(access_response)
if check_if_enrolled:
# Verify that the user is either enrolled in the course or a staff member.
# If user is not enrolled, raise UserNotEnrolled exception that will be caught by middleware.
if not ((user.id and CourseEnrollment.is_enrolled(user, course_key)) or has_access(user, 'staff', course)):
raise UserNotEnrolled(course_key)
return course
# Verify that the user is either enrolled in the course or a staff
# member. If user is not enrolled, raise UserNotEnrolled exception
# that will be caught by middleware.
if not ((user.id and CourseEnrollment.is_enrolled(user, course.id)) or has_access(user, 'staff', course)):
raise UserNotEnrolled(course.id)
def find_file(filesystem, dirs, filename):
......@@ -129,16 +157,6 @@ def find_file(filesystem, dirs, filename):
raise ResourceNotFoundError(u"Could not find {0}".format(filename))
def get_course_university_about_section(course): # pylint: disable=invalid-name
"""
Returns a snippet of HTML displaying the course's university.
Arguments:
course (CourseDescriptor|CourseOverview): A course.
"""
return course.display_org_with_default
def get_course_about_section(request, course, section_key):
"""
This returns the snippet of html to be rendered on the course about page,
......@@ -146,9 +164,6 @@ def get_course_about_section(request, course, section_key):
Valid keys:
- overview
- title
- university
- number
- short_description
- description
- key_dates (includes start, end, exams, etc)
......@@ -159,6 +174,7 @@ def get_course_about_section(request, course, section_key):
- syllabus
- textbook
- faq
- effort
- more_info
- ocw_links
"""
......@@ -167,7 +183,6 @@ def get_course_about_section(request, course, section_key):
# markup. This can change without effecting this interface when we find a
# good format for defining so many snippets of text/html.
# TODO: Remove number, instructors from this set
html_sections = {
'short_description',
'description',
......@@ -180,8 +195,6 @@ def get_course_about_section(request, course, section_key):
'textbook',
'faq',
'more_info',
'number',
'instructors',
'overview',
'effort',
'end_date',
......@@ -225,12 +238,6 @@ def get_course_about_section(request, course, section_key):
section_key, course.location.to_deprecated_string()
)
return None
elif section_key == "title":
return course.display_name_with_default
elif section_key == "university":
return get_course_university_about_section(course)
elif section_key == "number":
return course.display_number_with_default
raise KeyError("Invalid about key " + str(section_key))
......@@ -366,22 +373,6 @@ def get_course_syllabus_section(course, section_key):
raise KeyError("Invalid about key " + str(section_key))
def get_courses_by_university(user, domain=None):
'''
Returns dict of lists of courses available, keyed by course.org (ie university).
Courses are sorted by course.number.
'''
# TODO: Clean up how 'error' is done.
# filter out any courses that errored.
visible_courses = get_courses(user, domain)
universities = defaultdict(list)
for course in visible_courses:
universities[course.org].append(course)
return universities
def get_courses(user, domain=None):
'''
Returns a list of courses available, sorted by course.number
......@@ -400,6 +391,16 @@ def get_courses(user, domain=None):
return courses
def get_permission_for_course_about():
"""
Returns the CourseOverview object for the course after checking for access.
"""
return microsite.get_value(
'COURSE_ABOUT_VISIBILITY_PERMISSION',
settings.COURSE_ABOUT_VISIBILITY_PERMISSION
)
def sort_by_announcement(courses):
"""
Sorts a list of courses by their announcement date. If the date is
......
lms/djangoapps/courseware/management/commands/tests/test_dump_course.py
......@@ -153,7 +153,6 @@ class CommandsTestBase(ModuleStoreTestCase):
self.assertIn('children', element)
self.assertIn('category', element)
self.assertIn('inherited_metadata', element)
self.assertIsNone(element['inherited_metadata']['ispublic'])
# ... but does not contain inherited metadata containing a default value:
self.assertNotIn('due', element['inherited_metadata'])
......@@ -169,7 +168,6 @@ class CommandsTestBase(ModuleStoreTestCase):
self.assertIn('children', element)
self.assertIn('category', element)
self.assertIn('inherited_metadata', element)
self.assertIsNone(element['inherited_metadata']['ispublic'])
# ... and contains inherited metadata containing a default value:
self.assertIsNone(element['inherited_metadata']['due'])
......
lms/djangoapps/courseware/tests/helpers.py
......@@ -5,7 +5,7 @@ from django.core.urlresolvers import reverse
from django.test import TestCase
from django.test.client import RequestFactory
from courseware.access import has_access, COURSE_OVERVIEW_SUPPORTED_ACTIONS
from courseware.access import has_access
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from student.models import Registration
......@@ -151,30 +151,27 @@ class CourseAccessTestMixin(TestCase):
"""
Assert that a user has access to the given action for a given course.
Test with both the given course and, if the action is supported, with
a CourseOverview of the given course.
Test with both the given course and with a CourseOverview of the given
course.
Arguments:
user (User): a user.
action (str): type of access to test.
See access.py:COURSE_OVERVIEW_SUPPORTED_ACTIONS.
course (CourseDescriptor): a course.
"""
self.assertTrue(has_access(user, action, course))
if action in COURSE_OVERVIEW_SUPPORTED_ACTIONS:
self.assertTrue(has_access(user, action, CourseOverview.get_from_id(course.id)))
def assertCannotAccessCourse(self, user, action, course):
"""
Assert that a user lacks access to the given action the given course.
Test with both the given course and, if the action is supported, with
a CourseOverview of the given course.
Test with both the given course and with a CourseOverview of the given
course.
Arguments:
user (User): a user.
action (str): type of access to test.
See access.py:COURSE_OVERVIEW_SUPPORTED_ACTIONS.
course (CourseDescriptor): a course.
Note:
......@@ -184,5 +181,4 @@ class CourseAccessTestMixin(TestCase):
stack traces of failed tests easier to understand at a glance.
"""
self.assertFalse(has_access(user, action, course))
if action in COURSE_OVERVIEW_SUPPORTED_ACTIONS:
self.assertFalse(has_access(user, action, CourseOverview.get_from_id(course.id)))
lms/djangoapps/courseware/tests/test_access.py
......@@ -236,7 +236,7 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
mock_unit.start = start
self.verify_access(mock_unit, expected_access, expected_error_type)
def test__has_access_course_desc_can_enroll(self):
def test__has_access_course_can_enroll(self):
yesterday = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1)
tomorrow = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1)
......@@ -248,11 +248,11 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'), enrollment_domain=''
)
CourseEnrollmentAllowedFactory(email=user.email, course_id=course.id)
self.assertTrue(access._has_access_course_desc(user, 'enroll', course))
self.assertTrue(access._has_access_course(user, 'enroll', course))
# Staff can always enroll even outside the open enrollment period
user = StaffFactory.create(course_key=course.id)
self.assertTrue(access._has_access_course_desc(user, 'enroll', course))
self.assertTrue(access._has_access_course(user, 'enroll', course))
# Non-staff cannot enroll if it is between the start and end dates and invitation only
# and not specifically allowed
......@@ -262,7 +262,7 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
invitation_only=True
)
user = UserFactory.create()
self.assertFalse(access._has_access_course_desc(user, 'enroll', course))
self.assertFalse(access._has_access_course(user, 'enroll', course))
# Non-staff can enroll if it is between the start and end dates and not invitation only
course = Mock(
......@@ -270,7 +270,7 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'), enrollment_domain='',
invitation_only=False
)
self.assertTrue(access._has_access_course_desc(user, 'enroll', course))
self.assertTrue(access._has_access_course(user, 'enroll', course))
# Non-staff cannot enroll outside the open enrollment period if not specifically allowed
course = Mock(
......@@ -278,7 +278,7 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'), enrollment_domain='',
invitation_only=False
)
self.assertFalse(access._has_access_course_desc(user, 'enroll', course))
self.assertFalse(access._has_access_course(user, 'enroll', course))
def test__user_passed_as_none(self):
"""Ensure has_access handles a user being passed as null"""
......@@ -296,40 +296,30 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
id=course_id,
catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT
)
self.assertTrue(access._has_access_course_desc(user, 'see_in_catalog', course))
self.assertTrue(access._has_access_course_desc(user, 'see_about_page', course))
self.assertTrue(access._has_access_course_desc(staff, 'see_in_catalog', course))
self.assertTrue(access._has_access_course_desc(staff, 'see_about_page', course))
self.assertTrue(access._has_access_course(user, 'see_in_catalog', course))
self.assertTrue(access._has_access_course(user, 'see_about_page', course))
self.assertTrue(access._has_access_course(staff, 'see_in_catalog', course))
self.assertTrue(access._has_access_course(staff, 'see_about_page', course))
# Now set visibility to just about page
course = Mock(
id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'),
catalog_visibility=CATALOG_VISIBILITY_ABOUT
)
self.assertFalse(access._has_access_course_desc(user, 'see_in_catalog', course))
self.assertTrue(access._has_access_course_desc(user, 'see_about_page', course))
self.assertTrue(access._has_access_course_desc(staff, 'see_in_catalog', course))
self.assertTrue(access._has_access_course_desc(staff, 'see_about_page', course))
self.assertFalse(access._has_access_course(user, 'see_in_catalog', course))
self.assertTrue(access._has_access_course(user, 'see_about_page', course))
self.assertTrue(access._has_access_course(staff, 'see_in_catalog', course))
self.assertTrue(access._has_access_course(staff, 'see_about_page', course))
# Now set visibility to none, which means neither in catalog nor about pages
course = Mock(
id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'),
catalog_visibility=CATALOG_VISIBILITY_NONE
)
self.assertFalse(access._has_access_course_desc(user, 'see_in_catalog', course))
self.assertFalse(access._has_access_course_desc(user, 'see_about_page', course))
self.assertTrue(access._has_access_course_desc(staff, 'see_in_catalog', course))
self.assertTrue(access._has_access_course_desc(staff, 'see_about_page', course))
@ddt.data(True, False)
@patch.dict("django.conf.settings.FEATURES", {'ACCESS_REQUIRE_STAFF_FOR_COURSE': True})
def test_see_exists(self, ispublic):
"""
Test if user can see course
"""
user = UserFactory.create(is_staff=False)
course = Mock(ispublic=ispublic)
self.assertEquals(bool(access._has_access_course_desc(user, 'see_exists', course)), ispublic)
self.assertFalse(access._has_access_course(user, 'see_in_catalog', course))
self.assertFalse(access._has_access_course(user, 'see_about_page', course))
self.assertTrue(access._has_access_course(staff, 'see_in_catalog', course))
self.assertTrue(access._has_access_course(staff, 'see_about_page', course))
@patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True})
def test_access_on_course_with_pre_requisites(self):
......@@ -351,16 +341,16 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
# user should not be able to load course even if enrolled
CourseEnrollmentFactory(user=user, course_id=course.id)
response = access._has_access_course_desc(user, 'view_courseware_with_prerequisites', course)
response = access._has_access_course(user, 'view_courseware_with_prerequisites', course)
self.assertFalse(response)
self.assertIsInstance(response, access_response.MilestoneError)
# Staff can always access course
staff = StaffFactory.create(course_key=course.id)
self.assertTrue(access._has_access_course_desc(staff, 'view_courseware_with_prerequisites', course))
self.assertTrue(access._has_access_course(staff, 'view_courseware_with_prerequisites', course))
# User should be able access after completing required course
fulfill_course_milestone(pre_requisite_course.id, user)
self.assertTrue(access._has_access_course_desc(user, 'view_courseware_with_prerequisites', course))
self.assertTrue(access._has_access_course(user, 'view_courseware_with_prerequisites', course))
@ddt.data(
(True, True, True),
......@@ -377,10 +367,10 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
descriptor.mobile_available = mobile_available
self.assertEqual(
bool(access._has_access_course_desc(self.student, 'load_mobile', descriptor)),
bool(access._has_access_course(self.student, 'load_mobile', descriptor)),
student_expected
)
self.assertEqual(bool(access._has_access_course_desc(self.staff, 'load_mobile', descriptor)), staff_expected)
self.assertEqual(bool(access._has_access_course(self.staff, 'load_mobile', descriptor)), staff_expected)
@patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True})
def test_courseware_page_unfulfilled_prereqs(self):
......@@ -552,7 +542,6 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase):
user_attr_name (str): the name of the attribute on self that is the
User to test with.
action (str): action to test with.
See COURSE_OVERVIEW_SUPPORTED_ACTIONS for valid values.
course_attr_name (str): the name of the attribute on self that is
the CourseDescriptor to test with.
"""
......
lms/djangoapps/courseware/tests/test_courses.py
......@@ -18,7 +18,7 @@ from courseware.courses import (
get_course_info_section, get_course_about_section, get_cms_block_link
)
from courseware.courses import get_course_with_access
from courseware.courses import get_course_with_access, get_course_overview_with_access
from courseware.module_render import get_module_for_descriptor
from courseware.tests.helpers import get_request_for_user
from courseware.model_data import FieldDataCache
......@@ -30,7 +30,7 @@ from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.xml_importer import import_course_from_xml
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
from xmodule.tests.xml import factories as xml
from xmodule.tests.xml import XModuleXmlImportTest
......@@ -40,6 +40,7 @@ TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
@attr('shard_1')
@ddt.ddt
class CoursesTest(ModuleStoreTestCase):
"""Test methods related to fetching courses."""
......@@ -57,16 +58,28 @@ class CoursesTest(ModuleStoreTestCase):
cms_url = u"//{}/course/{}".format(CMS_BASE_TEST, unicode(self.course.location))
self.assertEqual(cms_url, get_cms_block_link(self.course, 'course'))
def test_get_course_with_access(self):
@ddt.data(get_course_with_access, get_course_overview_with_access)
def test_get_course_func_with_access_error(self, course_access_func):
user = UserFactory.create()
course = CourseFactory.create(visible_to_staff_only=True)
with self.assertRaises(CoursewareAccessException) as error:
get_course_with_access(user, 'load', course.id)
course_access_func(user, 'load', course.id)
self.assertEqual(error.exception.message, "Course not found.")
self.assertEqual(error.exception.access_response.error_code, "not_visible_to_user")
self.assertFalse(error.exception.access_response.has_access)
@ddt.data(
(get_course_with_access, 1),
(get_course_overview_with_access, 0),
)
@ddt.unpack
def test_get_course_func_with_access(self, course_access_func, num_mongo_calls):
user = UserFactory.create()
course = CourseFactory.create(emit_signals=True)
with check_mongo_calls(num_mongo_calls):
course_access_func(user, 'load', course.id)
@attr('shard_1')
class ModuleStoreBranchSettingTest(ModuleStoreTestCase):
......
lms/djangoapps/courseware/views.py
......@@ -37,11 +37,17 @@ from courseware.access import has_access, _adjust_start_date_for_beta_testers
from courseware.access_response import StartDateError
from courseware.access_utils import in_preview_mode
from courseware.courses import (
get_courses, get_course, get_course_by_id,
get_studio_url, get_course_with_access,
get_courses,
get_course,
get_course_by_id,
get_permission_for_course_about,
get_studio_url,
get_course_overview_with_access,
get_course_with_access,
sort_by_announcement,
sort_by_start_date,
UserNotEnrolled)
UserNotEnrolled
)
from courseware.masquerade import setup_masquerade
from openedx.core.djangoapps.credit.api import (
get_credit_requirement_status,
......@@ -802,11 +808,8 @@ def course_about(request, course_id):
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
with modulestore().bulk_operations(course_key):
permission_name = microsite.get_value(
'COURSE_ABOUT_VISIBILITY_PERMISSION',
settings.COURSE_ABOUT_VISIBILITY_PERMISSION
)
course = get_course_with_access(request.user, permission_name, course_key)
permission = get_permission_for_course_about()
course = get_course_with_access(request.user, permission, course_key)
if microsite.get_value('ENABLE_MKTG_SITE', settings.FEATURES.get('ENABLE_MKTG_SITE', False)):
return redirect(reverse('info', args=[course.id.to_deprecated_string()]))
......@@ -1066,7 +1069,7 @@ def submission_history(request, course_id, student_username, location):
except (InvalidKeyError, AssertionError):
return HttpResponse(escape(_(u'Invalid location.')))
course = get_course_with_access(request.user, 'load', course_key)
course = get_course_overview_with_access(request.user, 'load', course_key)
staff_access = bool(has_access(request.user, 'staff', course))
# Permission Denied if they don't have staff access and are trying to see
......
lms/djangoapps/django_comment_client/base/views.py
......@@ -15,7 +15,7 @@ from opaque_keys.edx.keys import CourseKey
from courseware.access import has_access
from util.file import store_uploaded_file
from courseware.courses import get_course_with_access, get_course_by_id
from courseware.courses import get_course_with_access, get_course_overview_with_access, get_course_by_id
import django_comment_client.settings as cc_settings
from django_comment_common.signals import (
thread_created,
......@@ -770,7 +770,7 @@ def users(request, course_id):
course_key = CourseKey.from_string(course_id)
try:
get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True)
get_course_overview_with_access(request.user, 'load', course_key, check_if_enrolled=True)
except Http404:
# course didn't exist, or requesting user does not have access to it.
return JsonError(status=404)
......
lms/djangoapps/lms_xblock/mixin.py
......@@ -65,11 +65,6 @@ class LmsBlockMixin(XBlockMixin):
scope=Scope.settings,
deprecated=True
)
ispublic = Boolean(
display_name=_("Course Is Public"),
help=_("Enter true or false. If true, the course is open to the public. If false, the course is open only to admins."),
scope=Scope.settings
)
visible_to_staff_only = Boolean(
help=_("If true, can be seen only by course staff, regardless of start date."),
default=False,
......
lms/djangoapps/mobile_api/users/serializers.py
......@@ -4,12 +4,9 @@ Serializer for user API
from rest_framework import serializers
from rest_framework.reverse import reverse
from django.template import defaultfilters
from courseware.access import has_access
from student.models import CourseEnrollment, User
from certificates.api import certificate_downloadable_status
from xmodule.course_module import DEFAULT_START_DATE
class CourseOverviewField(serializers.RelatedField):
......@@ -19,17 +16,6 @@ class CourseOverviewField(serializers.RelatedField):
def to_representation(self, course_overview):
course_id = unicode(course_overview.id)
if course_overview.advertised_start is not None:
start_type = 'string'
start_display = course_overview.advertised_start
elif course_overview.start != DEFAULT_START_DATE:
start_type = 'timestamp'
start_display = defaultfilters.date(course_overview.start, 'DATE_FORMAT')
else:
start_type = 'empty'
start_display = None
request = self.context.get('request')
return {
# identifiers
......@@ -40,8 +26,8 @@ class CourseOverviewField(serializers.RelatedField):
# dates
'start': course_overview.start,
'start_display': start_display,
'start_type': start_type,
'start_display': course_overview.start_display,
'start_type': course_overview.start_type,
'end': course_overview.end,
# notification info
......
lms/djangoapps/mobile_api/users/tests.py
......@@ -168,8 +168,10 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest
@ddt.data(
(NEXT_WEEK, ADVERTISED_START, ADVERTISED_START, "string"),
(NEXT_WEEK, None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp"),
(NEXT_WEEK, '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp"),
(DEFAULT_START_DATE, ADVERTISED_START, ADVERTISED_START, "string"),
(DEFAULT_START_DATE, None, None, "empty")
(DEFAULT_START_DATE, '', None, "empty"),
(DEFAULT_START_DATE, None, None, "empty"),
)
@ddt.unpack
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
......
lms/envs/common.py
......@@ -127,7 +127,6 @@ FEATURES = {
'DISABLE_LOGIN_BUTTON': False, # used in systems where login is automatic, eg MIT SSL
# extrernal access methods
'ACCESS_REQUIRE_STAFF_FOR_COURSE': False,
'AUTH_USE_OPENID': False,
'AUTH_USE_CERTIFICATES': False,
'AUTH_USE_OPENID_PROVIDER': False,
......
lms/envs/dev.py
......@@ -188,7 +188,6 @@ OPEN_ENDED_GRADING_INTERFACE = {
############################## LMS Migration ##################################
FEATURES['ENABLE_LMS_MIGRATION'] = True
FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = False # require that user be in the staff_* group to be able to enroll
FEATURES['XQA_SERVER'] = 'http://xqa:server@content-qa.edX.mit.edu/xqa'
INSTALLED_APPS += ('lms_migration',)
......
lms/templates/course.html
......@@ -2,30 +2,28 @@
<%!
from django.utils.translation import ugettext as _
from django.core.urlresolvers import reverse
from courseware.courses import get_course_about_section
from openedx.core.lib.courses import course_image_url
%>
<%page args="course" />
<article class="course" id="${course.id | h}" role="region" aria-label="${get_course_about_section(request, course, 'title')}">
<article class="course" id="${course.id | h}" role="region" aria-label="${course.display_name_with_default}">
<a href="${reverse('about_course', args=[course.id.to_deprecated_string()])}">
<header class="course-image">
<div class="cover-image">
<img src="${course_image_url(course)}" alt="${get_course_about_section(request, course, 'title')} ${course.display_number_with_default}" />
<img src="${course.course_image_url | h}" alt="${course.display_name_with_default} ${course.display_number_with_default | h}" />
<div class="learn-more" aria-hidden=true>${_("LEARN MORE")}</div>
</div>
</header>
<div class="course-info" aria-hidden="true">
<h2 class="course-name">
<span class="course-organization">${get_course_about_section(request, course, 'university')}</span>
<span class="course-code">${course.display_number_with_default}</span>
<span class="course-title">${get_course_about_section(request, course, 'title')}</span>
<span class="course-organization">${course.display_org_with_default | h}</span>
<span class="course-code">${course.display_number_with_default | h}</span>
<span class="course-title">${course.display_name_with_default}</span>
</h2>
<div class="course-date" aria-hidden="true">${_("Starts")}: ${course.start_datetime_text()}</div>
</div>
<div class="sr">
<ul>
<li>${get_course_about_section(request, course, 'university')}</li>
<li>${course.display_number_with_default}</li>
<li>${course.display_org_with_default | h}</li>
<li>${course.display_number_with_default | h}</li>
<li>${_("Starts")}: <time itemprop="startDate" datetime="${course.start_datetime_text()}">${course.start_datetime_text()}</time></li>
</ul>
</div>
......
lms/templates/courseware/course_about.html
......@@ -13,7 +13,7 @@ from openedx.core.lib.courses import course_image_url
<%block name="headextra">
## OG (Open Graph) title and description added below to give social media info to display
## (https://developers.facebook.com/docs/opengraph/howtos/maximizing-distribution-media-content#tags)
<meta property="og:title" content="${get_course_about_section(request, course, 'title')}" />
<meta property="og:title" content="${course.display_name_with_default}" />
<meta property="og:description" content="${get_course_about_section(request, course, 'short_description')}" />
</%block>
......@@ -102,7 +102,7 @@ from openedx.core.lib.courses import course_image_url
<script src="${static.url('js/course_info.js')}"></script>
</%block>
<%block name="pagetitle">${get_course_about_section(request, course, "title")}</%block>
<%block name="pagetitle">${course.display_name_with_default}</%block>
<section class="course-info">
<header class="course-profile">
......@@ -111,9 +111,9 @@ from openedx.core.lib.courses import course_image_url
<section class="intro">
<hgroup>
<h1>
${get_course_about_section(request, course, "title")}
${course.display_name_with_default}
% if not self.theme_enabled():
<a href="#">${get_course_about_section(request, course, "university")}</a>
<a href="#">${course.display_org_with_default | h}</a>
% endif
</h1>
</hgroup>
......@@ -220,10 +220,10 @@ from openedx.core.lib.courses import course_image_url
## or something allowing themes to do whatever they
## want here (and on this whole page, really).
% if self.stanford_theme_enabled():
<a href="http://twitter.com/intent/tweet?text=I+just+enrolled+in+${course.number}+${get_course_about_section(request, course, 'title')}!+(http://class.stanford.edu)" class="share">
<a href="http://twitter.com/intent/tweet?text=I+just+enrolled+in+${course.number}+${course.display_name_with_default}!+(http://class.stanford.edu)" class="share">
<i class="icon fa fa-twitter"></i><span class="sr">${_("Tweet that you've enrolled in this course")}</span>
</a>
<a href="mailto:?subject=Take%20a%20course%20at%20Stanford%20online!&body=I%20just%20enrolled%20in%20${course.number}%20${get_course_about_section(request, course, 'title')}+(http://class.stanford.edu)" class="share">
<a href="mailto:?subject=Take%20a%20course%20at%20Stanford%20online!&body=I%20just%20enrolled%20in%20${course.number}%20${course.display_name_with_default}+(http://class.stanford.edu)" class="share">
<i class="icon fa fa-envelope"></i><span class="sr">${_("Email someone to say you've enrolled in this course")}</span>
</a>
% else:
......@@ -235,7 +235,7 @@ from openedx.core.lib.courses import course_image_url
## Twitter account. {url} should appear at the end of the text.
tweet_text = _("I just enrolled in {number} {title} through {account}: {url}").format(
number=course.number,
title=get_course_about_section(request, course, 'title'),
title=course.display_name_with_default,
account=microsite.get_value('course_about_twitter_account', settings.PLATFORM_TWITTER_ACCOUNT),
url=u"http://{domain}{path}".format(
domain=site_domain,
......@@ -250,7 +250,7 @@ from openedx.core.lib.courses import course_image_url
subject=_("Take a course with {platform} online").format(platform=platform_name),
body=_("I just enrolled in {number} {title} through {platform} {url}").format(
number=course.number,
title=get_course_about_section(request, course, 'title'),
title=course.display_name_with_default,
platform=platform_name,
url=u"http://{domain}{path}".format(
domain=site_domain,
......
lms/templates/dashboard/_dashboard_course_listing.html
......@@ -7,7 +7,6 @@ from django.utils.translation import ugettext as _
from django.utils.translation import ungettext
from django.core.urlresolvers import reverse
from markupsafe import escape
from courseware.courses import get_course_university_about_section
from course_modes.models import CourseMode
from course_modes.helpers import enrollment_mode_display
from student.helpers import (
......@@ -99,7 +98,7 @@ from student.helpers import (
% endif
</h3>
<div class="course-info">
<span class="info-university">${get_course_university_about_section(course_overview)} - </span>
<span class="info-university">${course_overview.display_org_with_default | h} - </span>
<span class="info-course-id">${course_overview.display_number_with_default | h}</span>
<span class="info-date-block" data-tooltip="Hi">
% if course_overview.has_ended():
......
lms/templates/shoppingcart/receipt.html
......@@ -3,7 +3,6 @@
from django.utils.translation import ugettext as _
from django.utils.translation import ungettext
from django.core.urlresolvers import reverse
from courseware.courses import get_course_about_section, get_course_by_id
from markupsafe import escape
from microsite_configuration import microsite
from openedx.core.lib.courses import course_image_url
......@@ -293,7 +292,7 @@ from openedx.core.lib.courses import course_image_url
<div class="clearfix">
<div class="image">
<img class="item-image" src="${course_image_url(course)}"
alt="${course.display_number_with_default | h} ${get_course_about_section(request, course, 'title')} Image"/>
alt="${course.display_number_with_default | h} ${course.display_name_with_default} Image"/>
</div>
<div class="data-input">
......
lms/templates/shoppingcart/registration_code_receipt.html
<%!
from django.utils.translation import ugettext as _
from django.core.urlresolvers import reverse
from courseware.courses import get_course_about_section
from openedx.core.lib.courses import course_image_url
%>
<%inherit file="../main.html" />
......@@ -21,7 +20,7 @@ from openedx.core.lib.courses import course_image_url
<img class="item-image" src="${course_image_url(course)}"
alt="${_("{course_number} {course_title} Cover Image").format(
course_number=course.display_number_with_default,
course_title=get_course_about_section(request, course, 'title'),
course_title=course.display_name_with_default,
)}"/>
</div>
<div class="enrollment-details">
......
lms/templates/shoppingcart/registration_code_redemption.html
<%!
from django.utils.translation import ugettext as _
from django.core.urlresolvers import reverse
from courseware.courses import get_course_about_section
from openedx.core.lib.courses import course_image_url
%>
<%inherit file="../main.html" />
......@@ -21,7 +20,7 @@ from openedx.core.lib.courses import course_image_url
<img class="item-image" src="${course_image_url(course)}"
alt="${_("{course_number} {course_title} Cover Image").format(
course_number=course.display_number_with_default,
course_title=get_course_about_section(request, course, 'title'),
course_title=course.display_name_with_default,
)}" />
</div>
<div class="enrollment-details">
......
lms/templates/shoppingcart/shopping_cart.html
......@@ -2,7 +2,6 @@
<%block name="review_highlight">class="active"</%block>
<%!
from courseware.courses import get_course_about_section
from django.core.urlresolvers import reverse
from edxmako.shortcuts import marketing_link
from django.utils.translation import ugettext as _
......@@ -67,7 +66,7 @@ from openedx.core.lib.courses import course_image_url
<div class="clearfix">
<div class="image">
<img class="item-image" src="${course_image_url(course)}"
alt="${course.display_number_with_default | h} ${get_course_about_section(request, course, 'title')} ${_('Cover Image')}" />
alt="${course.display_number_with_default | h} ${course.display_name_with_default} ${_('Cover Image')}" />
</div>
<div class="data-input">
## Translators: "Registration for:" is followed by a course name
......
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:
......@@ -47,14 +49,4 @@ class Command(BaseCommand):
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]},
)
openedx/core/djangoapps/models/course_details.py
......@@ -60,10 +60,13 @@ class CourseDetails(object):
self.self_paced = None
@classmethod
def _fetch_about_attribute(cls, course_key, attribute):
def fetch_about_attribute(cls, course_key, attribute):
"""
Retrieve an attribute from a course's "about" info
"""
if attribute not in ABOUT_ATTRIBUTES + ['video']:
raise ValueError("'{0}' is not a valid course about attribute.".format(attribute))
usage_key = course_key.make_usage_key('about', attribute)
try:
value = modulestore().get_item(usage_key).data
......@@ -96,7 +99,7 @@ class CourseDetails(object):
course_details.intro_video = cls.fetch_youtube_video_id(course_key)
for attribute in ABOUT_ATTRIBUTES:
value = cls._fetch_about_attribute(course_key, attribute)
value = cls.fetch_about_attribute(course_key, attribute)
if value is not None:
setattr(course_details, attribute, value)
......@@ -107,7 +110,7 @@ class CourseDetails(object):
"""
Returns the course about video ID.
"""
raw_video = cls._fetch_about_attribute(course_key, 'video')
raw_video = cls.fetch_about_attribute(course_key, 'video')
if raw_video:
return cls.parse_video_tag(raw_video)
......@@ -121,13 +124,6 @@ class CourseDetails(object):
return "http://www.youtube.com/watch?v={0}".format(video_id)
@classmethod
def fetch_effort(cls, course_key):
"""
Returns the hours per week of effort for the course.
"""
return cls._fetch_about_attribute(course_key, 'effort')
@classmethod
def update_about_item(cls, course, about_key, data, user_id, store=None):
"""
Update the about item with the new data blob. If data is None,
......
openedx/core/djangoapps/models/tests/test_course_details.py
......@@ -3,6 +3,7 @@ Tests for CourseDetails
"""
import datetime
import ddt
from django.utils.timezone import UTC
from xmodule.modulestore import ModuleStoreEnum
......@@ -10,9 +11,10 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
from openedx.core.djangoapps.models.course_details import CourseDetails
from openedx.core.djangoapps.models.course_details import CourseDetails, ABOUT_ATTRIBUTES
@ddt.ddt
class CourseDetailsTestCase(ModuleStoreTestCase):
"""
Tests the first course settings page (course dates, overview, etc.).
......@@ -111,11 +113,19 @@ class CourseDetailsTestCase(ModuleStoreTestCase):
)
self.assertFalse(updated_details.self_paced)
def test_fetch_effort(self):
effort_value = 'test_hours_of_effort'
@ddt.data(*ABOUT_ATTRIBUTES)
def test_fetch_about_attribute(self, attribute_name):
attribute_value = 'test_value'
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
CourseDetails.update_about_item(self.course, 'effort', effort_value, self.user.id)
self.assertEqual(CourseDetails.fetch_effort(self.course.id), effort_value)
CourseDetails.update_about_item(self.course, attribute_name, attribute_value, self.user.id)
self.assertEqual(CourseDetails.fetch_about_attribute(self.course.id, attribute_name), attribute_value)
def test_fetch_about_attribute_error(self):
attribute_name = 'not_an_about_attribute'
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
CourseDetails.update_about_item(self.course, attribute_name, 'test_value', self.user.id)
with self.assertRaises(ValueError):
CourseDetails.fetch_about_attribute(self.course.id, attribute_name)
def test_fetch_video(self):
video_value = 'test_video_id'
......
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