diff --git a/common/djangoapps/enrollment/data.py b/common/djangoapps/enrollment/data.py
index f58ac8c0e32c38626bbbac234a377490d2ae83e6..81d896d038f948e8f0a0d652f451605ca31ca650 100644
--- a/common/djangoapps/enrollment/data.py
+++ b/common/djangoapps/enrollment/data.py
@@ -1,22 +1,24 @@
"""
Data Aggregation Layer of the Enrollment API. Collects all enrollment specific data into a single
source to be used throughout the API.
-
"""
import logging
+
from django.contrib.auth.models import User
from opaque_keys.edx.keys import CourseKey
-from xmodule.modulestore.django import modulestore
+
from enrollment.errors import (
CourseNotFoundError, CourseEnrollmentClosedError, CourseEnrollmentFullError,
CourseEnrollmentExistsError, UserNotFoundError, InvalidEnrollmentAttribute
)
from enrollment.serializers import CourseEnrollmentSerializer, CourseField
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from student.models import (
CourseEnrollment, NonExistentCourseError, EnrollmentClosedError,
CourseFullError, AlreadyEnrolledError, CourseEnrollmentAttribute
)
+
log = logging.getLogger(__name__)
@@ -245,7 +247,7 @@ def _invalid_attribute(attributes):
def get_course_enrollment_info(course_id, include_expired=False):
"""Returns all course enrollment information for the given course.
- Based on the course id, return all related course information..
+ Based on the course id, return all related course information.
Args:
course_id (str): The course to retrieve enrollment information for.
@@ -261,9 +263,12 @@ def get_course_enrollment_info(course_id, include_expired=False):
"""
course_key = CourseKey.from_string(course_id)
- course = modulestore().get_course(course_key)
- if course is None:
+
+ try:
+ course = CourseOverview.get_from_id(course_key)
+ except CourseOverview.DoesNotExist:
msg = u"Requested enrollment information for unknown course {course}".format(course=course_id)
log.warning(msg)
raise CourseNotFoundError(msg)
- return CourseField().to_native(course, include_expired=include_expired)
+ else:
+ return CourseField().to_native(course, include_expired=include_expired)
diff --git a/common/djangoapps/enrollment/serializers.py b/common/djangoapps/enrollment/serializers.py
index 4513b4ed955f6f07e9f5b65fcbf81e57098f738a..d9e6ce791834c8593981daaca003ec63390b7de7 100644
--- a/common/djangoapps/enrollment/serializers.py
+++ b/common/djangoapps/enrollment/serializers.py
@@ -1,12 +1,12 @@
"""
Serializers for all Course Enrollment related return objects.
-
"""
import logging
from rest_framework import serializers
-from student.models import CourseEnrollment
+
from course_modes.models import CourseMode
+from student.models import CourseEnrollment
log = logging.getLogger(__name__)
@@ -39,19 +39,18 @@ class CourseField(serializers.RelatedField):
"""
def to_native(self, course, **kwargs):
- course_id = unicode(course.id)
course_modes = ModeSerializer(
CourseMode.modes_for_course(course.id, kwargs.get('include_expired', False), only_selectable=False)
).data # pylint: disable=no-member
return {
- "course_id": course_id,
- "enrollment_start": course.enrollment_start,
- "enrollment_end": course.enrollment_end,
- "course_start": course.start,
- "course_end": course.end,
- "invite_only": course.invitation_only,
- "course_modes": course_modes,
+ 'course_id': unicode(course.id),
+ 'enrollment_start': course.enrollment_start,
+ 'enrollment_end': course.enrollment_end,
+ 'course_start': course.start,
+ 'course_end': course.end,
+ 'invite_only': course.invitation_only,
+ 'course_modes': course_modes,
}
diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py
index a648e1742c02ded502cd9fdaf1bc5ac23eb0e7f7..e8cd16b936a7037c2ea35f722e84fd98143cae44 100644
--- a/common/djangoapps/enrollment/tests/test_views.py
+++ b/common/djangoapps/enrollment/tests/test_views.py
@@ -16,7 +16,7 @@ from rest_framework.test import APITestCase
from rest_framework import status
from django.conf import settings
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls_range
from django.test.utils import override_settings
from course_modes.models import CourseMode
@@ -26,8 +26,9 @@ from util.models import RateLimitConfiguration
from util.testing import UrlResetMixin
from enrollment import api
from enrollment.errors import CourseEnrollmentError
-from openedx.core.lib.django_test_client_utils import get_absolute_url
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.user_api.models import UserOrgTag
+from openedx.core.lib.django_test_client_utils import get_absolute_url
from student.tests.factories import UserFactory, CourseModeFactory
from student.models import CourseEnrollment
from embargo.test_utils import restrict_course
@@ -47,6 +48,8 @@ class EnrollmentTestMixin(object):
mode=CourseMode.HONOR,
is_active=None,
enrollment_attributes=None,
+ min_mongo_calls=0,
+ max_mongo_calls=0,
):
"""
Enroll in the course and verify the response's status code. If the expected status is 200, also validates
@@ -77,31 +80,33 @@ class EnrollmentTestMixin(object):
if as_server:
extra['HTTP_X_EDX_API_KEY'] = self.API_KEY
- with patch('enrollment.views.audit_log') as mock_audit_log:
- url = reverse('courseenrollments')
- response = self.client.post(url, json.dumps(data), content_type='application/json', **extra)
- self.assertEqual(response.status_code, expected_status)
+ # Verify that the modulestore is queried as expected.
+ with check_mongo_calls_range(min_finds=min_mongo_calls, max_finds=max_mongo_calls):
+ with patch('enrollment.views.audit_log') as mock_audit_log:
+ url = reverse('courseenrollments')
+ response = self.client.post(url, json.dumps(data), content_type='application/json', **extra)
+ self.assertEqual(response.status_code, expected_status)
- if expected_status == status.HTTP_200_OK:
- data = json.loads(response.content)
- self.assertEqual(course_id, data['course_details']['course_id'])
+ if expected_status == status.HTTP_200_OK:
+ data = json.loads(response.content)
+ self.assertEqual(course_id, data['course_details']['course_id'])
- if mode is not None:
- self.assertEqual(mode, data['mode'])
+ if mode is not None:
+ self.assertEqual(mode, data['mode'])
- if is_active is not None:
- self.assertEqual(is_active, data['is_active'])
- else:
- self.assertTrue(data['is_active'])
+ if is_active is not None:
+ self.assertEqual(is_active, data['is_active'])
+ else:
+ self.assertTrue(data['is_active'])
- if as_server:
- # Verify that an audit message was logged.
- self.assertTrue(mock_audit_log.called)
+ if as_server:
+ # Verify that an audit message was logged.
+ self.assertTrue(mock_audit_log.called)
- # If multiple enrollment calls are made in the scope of a
- # single test, we want to validate that audit messages are
- # logged for each call.
- mock_audit_log.reset_mock()
+ # If multiple enrollment calls are made in the scope of a
+ # single test, we want to validate that audit messages are
+ # logged for each call.
+ mock_audit_log.reset_mock()
return response
@@ -141,6 +146,10 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase):
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)
+
self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD)
self.other_user = UserFactory.create()
self.client.login(username=self.USERNAME, password=self.PASSWORD)
@@ -382,6 +391,10 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase):
@ddt.unpack
def test_get_course_details_course_dates(self, start_datetime, end_datetime, expected_start, expected_end):
course = CourseFactory.create(start=start_datetime, end=end_datetime)
+ # 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(course.id)
+
self.assert_enrollment_status(course_id=unicode(course.id))
# Check course details
@@ -411,7 +424,12 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase):
self.assertEqual(data[0]['course_details']['course_end'], expected_end)
def test_with_invalid_course_id(self):
- self.assert_enrollment_status(course_id='entirely/fake/course', expected_status=status.HTTP_400_BAD_REQUEST)
+ self.assert_enrollment_status(
+ course_id='entirely/fake/course',
+ expected_status=status.HTTP_400_BAD_REQUEST,
+ min_mongo_calls=3,
+ max_mongo_calls=4
+ )
def test_get_enrollment_details_bad_course(self):
resp = self.client.get(
@@ -797,7 +815,12 @@ class EnrollmentEmbargoTest(EnrollmentTestMixin, UrlResetMixin, ModuleStoreTestC
def setUp(self):
""" Create a course and user, then log in. """
super(EnrollmentEmbargoTest, self).setUp('embargo')
+
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)
+
self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD)
self.client.login(username=self.USERNAME, password=self.PASSWORD)
self.url = reverse('courseenrollments')
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index 686874ca9ed6ea914798b117e3aa6c09c89de495..ff9a863c3322e8602a8513cf43c42a0309e423cc 100644
--- a/common/djangoapps/student/models.py
+++ b/common/djangoapps/student/models.py
@@ -10,16 +10,19 @@ file and check it in at the same time as your model changes. To do that,
2. ./manage.py lms schemamigration student --auto description_of_your_change
3. Add the migration file created in edx-platform/common/djangoapps/student/migrations/
"""
+from collections import defaultdict, OrderedDict
from datetime import datetime, timedelta
+from functools import total_ordering
import hashlib
+from importlib import import_module
import json
import logging
from pytz import UTC
-import uuid
-from collections import defaultdict, OrderedDict
-import dogstats_wrapper as dog_stats_api
from urllib import urlencode
+import uuid
+import analytics
+from config_models.models import ConfigurationModel
from django.utils.translation import ugettext_lazy as _
from django.conf import settings
from django.utils import timezone
@@ -33,28 +36,21 @@ from django.dispatch import receiver, Signal
from django.core.exceptions import ObjectDoesNotExist
from django.utils.translation import ugettext_noop
from django_countries.fields import CountryField
-from config_models.models import ConfigurationModel
-from track import contexts
+import dogstats_wrapper as dog_stats_api
from eventtracking import tracker
-from importlib import import_module
-
-from south.modelsinspector import add_introspection_rules
-
+from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey
-
-import lms.lib.comment_client as cc
-from util.model_utils import emit_field_changed_events, get_changed_fields_dict
-from util.query import use_read_replica_if_available
+from south.modelsinspector import add_introspection_rules
+from track import contexts
from xmodule_django.models import CourseKeyField, NoneToEmptyManager
-from xmodule.modulestore.exceptions import ItemNotFoundError
-from xmodule.modulestore.django import modulestore
-from opaque_keys.edx.keys import CourseKey
-from functools import total_ordering
from certificates.models import GeneratedCertificate
from course_modes.models import CourseMode
+import lms.lib.comment_client as cc
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+from util.model_utils import emit_field_changed_events, get_changed_fields_dict
+from util.query import use_read_replica_if_available
-import analytics
UNENROLL_DONE = Signal(providing_args=["course_enrollment", "skip_refund"])
log = logging.getLogger(__name__)
@@ -1064,18 +1060,15 @@ class CourseEnrollment(models.Model):
"""
# All the server-side checks for whether a user is allowed to enroll.
try:
- course = modulestore().get_course(course_key)
- except ItemNotFoundError:
- log.warning(
- u"User %s failed to enroll in non-existent course %s",
- user.username,
- course_key.to_deprecated_string(),
- )
- raise NonExistentCourseError
+ course = CourseOverview.get_from_id(course_key)
+ except CourseOverview.DoesNotExist:
+ # This is here to preserve legacy behavior which allowed enrollment in courses
+ # announced before the start of content creation.
+ if check_access:
+ log.warning(u"User %s failed to enroll in non-existent course %s", user.username, unicode(course_key))
+ raise NonExistentCourseError
if check_access:
- if course is None:
- raise NonExistentCourseError
if CourseEnrollment.is_enrollment_closed(user, course):
log.warning(
u"User %s failed to enroll in course %s because enrollment is closed",
@@ -1320,7 +1313,8 @@ class CourseEnrollment(models.Model):
@property
def course(self):
- return modulestore().get_course(self.course_id)
+ # Deprecated. Please use the `course_overview` property instead.
+ return self.course_overview
@property
def course_overview(self):
@@ -1334,7 +1328,6 @@ class CourseEnrollment(models.Model):
become stale.
"""
if not self._course_overview:
- from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
try:
self._course_overview = CourseOverview.get_from_id(self.course_id)
except (CourseOverview.DoesNotExist, IOError):
diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py
index cdf75a342b143a6ba8b6e3285dec9bcae9aa4bb4..362d85b95e8e3c25aff87e83df6413fbb974ec13 100644
--- a/common/djangoapps/student/tests/test_course_listing.py
+++ b/common/djangoapps/student/tests/test_course_listing.py
@@ -2,26 +2,27 @@
Unit tests for getting the list of courses for a user through iterating all courses and
by reversing group name formats.
"""
+import unittest
+
+from django.conf import settings
+from django.test.client import Client
import mock
-from mock import patch, Mock
-from student.tests.factories import UserFactory
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+from student.models import CourseEnrollment
from student.roles import GlobalStaff
+from student.tests.factories import UserFactory
+from student.views import get_course_enrollments
+from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
-from xmodule.modulestore.django import modulestore
-from xmodule.error_module import ErrorDescriptor
-from django.test.client import Client
-from student.models import CourseEnrollment
-from student.views import get_course_enrollments
from util.milestones_helpers import (
get_pre_requisite_courses_not_completed,
set_prerequisite_courses,
seed_milestone_relationship_types
)
-import unittest
-from django.conf import settings
class TestCourseListing(ModuleStoreTestCase):
@@ -91,10 +92,12 @@ class TestCourseListing(ModuleStoreTestCase):
course_key = mongo_store.make_course_key('Org1', 'Course1', 'Run1')
self._create_course_with_access_groups(course_key, default_store=ModuleStoreEnum.Type.mongo)
- with patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', Mock(side_effect=Exception)):
+ with mock.patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', mock.Mock(side_effect=Exception)):
self.assertIsInstance(modulestore().get_course(course_key), ErrorDescriptor)
- # get courses through iterating all courses
+ # Invalidate (e.g., delete) the corresponding CourseOverview, forcing get_course to be called.
+ CourseOverview.objects.filter(id=course_key).delete()
+
courses_list = list(get_course_enrollments(self.student, None, []))
self.assertEqual(courses_list, [])
diff --git a/common/djangoapps/student/tests/test_recent_enrollments.py b/common/djangoapps/student/tests/test_recent_enrollments.py
index 07516af01a4e0d82c165cb36de597987416dec81..2fd54d87bfed1b3be3ebf1078c87ff0617a1910a 100644
--- a/common/djangoapps/student/tests/test_recent_enrollments.py
+++ b/common/djangoapps/student/tests/test_recent_enrollments.py
@@ -112,10 +112,10 @@ class TestRecentEnrollments(ModuleStoreTestCase):
recent_course_list = _get_recently_enrolled_courses(courses_list)
self.assertEqual(len(recent_course_list), 5)
- self.assertEqual(recent_course_list[1].course, courses[0])
- self.assertEqual(recent_course_list[2].course, courses[1])
- self.assertEqual(recent_course_list[3].course, courses[2])
- self.assertEqual(recent_course_list[4].course, courses[3])
+ self.assertEqual(recent_course_list[1].course.id, courses[0].id)
+ self.assertEqual(recent_course_list[2].course.id, courses[1].id)
+ self.assertEqual(recent_course_list[3].course.id, courses[2].id)
+ self.assertEqual(recent_course_list[4].course.id, courses[3].id)
def test_dashboard_rendering(self):
"""
diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py
index 00af8014b5f11e838b13745c8313af1319be53e1..42842cab37395aaa8490c3c5941fca0c382af49a 100644
--- a/common/djangoapps/student/tests/tests.py
+++ b/common/djangoapps/student/tests/tests.py
@@ -494,9 +494,9 @@ class DashboardTest(ModuleStoreTestCase):
"""
Check that the student dashboard makes use of course metadata caching.
- The first time the student dashboard displays a specific course, it will
- make a call to the module store. After that first request, though, the
- course's metadata should be cached as a CourseOverview.
+ After enrolling a student in a course, that course's metadata should be
+ cached as a CourseOverview. The student dashboard should never have to make
+ calls to the modulestore.
Arguments:
modulestore_type (ModuleStoreEnum.Type): Type of modulestore to create
@@ -511,23 +511,21 @@ class DashboardTest(ModuleStoreTestCase):
involve adding fields to CourseOverview so that loading a full
CourseDescriptor isn't necessary.
"""
- # Create a course, log in the user, and enroll them in the course.
+ # Create a course and log in the user.
test_course = CourseFactory.create(default_store=modulestore_type)
self.client.login(username="jack", password="test")
- CourseEnrollment.enroll(self.user, test_course.id)
- # The first request will result in a modulestore query.
+ # Enrolling the user in the course will result in a modulestore query.
with check_mongo_calls(expected_mongo_calls):
- response_1 = self.client.get(reverse('dashboard'))
- self.assertEquals(response_1.status_code, 200)
+ CourseEnrollment.enroll(self.user, test_course.id)
# Subsequent requests will only result in SQL queries to load the
# CourseOverview object that has been created.
with check_mongo_calls(0):
+ response_1 = self.client.get(reverse('dashboard'))
+ self.assertEquals(response_1.status_code, 200)
response_2 = self.client.get(reverse('dashboard'))
self.assertEquals(response_2.status_code, 200)
- response_3 = self.client.get(reverse('dashboard'))
- self.assertEquals(response_3.status_code, 200)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@patch.dict(settings.FEATURES, {"IS_EDX_DOMAIN": True})
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 853fd93036d9f852a17abd1d151cbf68e4da725e..3017f99d19900ec95eb53bb75af2c94c316b8e9c 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -1353,7 +1353,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
otherwise a publish will be signalled at the end of the bulk operation
Arguments:
- library_updated - library_updated to which the signal applies
+ library_key - library_key to which the signal applies
"""
signal_handler = getattr(self, 'signal_handler', None)
if signal_handler:
@@ -1363,6 +1363,14 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
else:
signal_handler.send("library_updated", library_key=library_key)
+ def _emit_course_deleted_signal(self, course_key):
+ """
+ Helper method used to emit the course_deleted signal.
+ """
+ signal_handler = getattr(self, 'signal_handler', None)
+ if signal_handler:
+ signal_handler.send("course_deleted", course_key=course_key)
+
def only_xmodules(identifier, entry_points):
"""Only use entry_points that are supplied by the xmodule package"""
diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py
index 8c631a75aebd83b6cd90091520f38b53fd4b245e..8b91916b4590b30cdc4bd78bd224a653ef2f0f5c 100644
--- a/common/lib/xmodule/xmodule/modulestore/django.py
+++ b/common/lib/xmodule/xmodule/modulestore/django.py
@@ -87,11 +87,13 @@ class SignalHandler(object):
do the actual work.
"""
course_published = django.dispatch.Signal(providing_args=["course_key"])
+ course_deleted = django.dispatch.Signal(providing_args=["course_key"])
library_updated = django.dispatch.Signal(providing_args=["library_key"])
_mapping = {
"course_published": course_published,
- "library_updated": library_updated
+ "course_deleted": course_deleted,
+ "library_updated": library_updated,
}
def __init__(self, modulestore_class):
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index 8c181b2639e77a61857bdfe09ccf4fd6e67dce9a..6e6335d3bdd2fab644ca050f5d2f6aca57a88a6c 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
@@ -167,6 +167,8 @@ class DraftModuleStore(MongoModuleStore):
self.collection.remove(course_query, multi=True)
self.delete_all_asset_metadata(course_key, user_id)
+ self._emit_course_deleted_signal(course_key)
+
def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, **kwargs):
"""
Only called if cloning within this store or if env doesn't set up mixed.
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index f29a0b04a10a5a95edfe3a377bff5f9e6767acbe..3a829212c0ec3cce7fafe89ef5a1e18fe8ca2cbf 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -2443,6 +2443,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
# in case the course is later restored.
# super(SplitMongoModuleStore, self).delete_course(course_key, user_id)
+ self._emit_course_deleted_signal(course_key)
+
@contract(block_map="dict(BlockKey: dict)", block_key=BlockKey)
def inherit_settings(
self, block_map, block_key, inherited_settings_map, inheriting_settings=None, inherited_from=None
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
index 1872a481d1f8bdb8673fac66fed41619f2ed5c37..74aa23066b78cce8b39ac5286e84fb17f3de754e 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
@@ -2414,6 +2414,34 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
self.assertEqual(receiver.call_count, 0)
self.assertEqual(receiver.call_count, 0)
+ @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+ def test_course_deleted_signal(self, default):
+ with MongoContentstoreBuilder().build() as contentstore:
+ self.store = MixedModuleStore(
+ contentstore=contentstore,
+ create_modulestore_instance=create_modulestore_instance,
+ mappings={},
+ signal_handler=SignalHandler(MixedModuleStore),
+ **self.OPTIONS
+ )
+ self.addCleanup(self.store.close_all_connections)
+
+ with self.store.default_store(default):
+ self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler)
+
+ with mock_signal_receiver(SignalHandler.course_deleted) as receiver:
+ self.assertEqual(receiver.call_count, 0)
+
+ # Create a course
+ course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id)
+ course_key = course.id
+
+ # Delete the course
+ course = self.store.delete_course(course_key, self.user_id)
+
+ # Verify that the signal was emitted
+ self.assertEqual(receiver.call_count, 1)
+
@ddt.ddt
@attr('mongo')
diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py
index a4735f9d13d4543dad23b34d3725a55ef4426861..e544afab5ed6de9320183d5e7d4fc20a9aba9964 100644
--- a/lms/djangoapps/courseware/access.py
+++ b/lms/djangoapps/courseware/access.py
@@ -10,8 +10,8 @@ Note: The access control logic in this file does NOT check for enrollment in
If enrollment is to be checked, use get_course_with_access in courseware.courses.
It is a wrapper around has_access that additionally checks for enrollment.
"""
-import logging
from datetime import datetime, timedelta
+import logging
import pytz
from django.conf import settings
@@ -245,6 +245,58 @@ def _can_load_course_on_mobile(user, course):
)
+def _can_enroll_courselike(user, courselike):
+ """
+ Ascertain if the user can enroll in the given courselike object.
+
+ Arguments:
+ user (User): The user attempting to enroll.
+ courselike (CourseDescriptor or CourseOverview): The object representing the
+ course in which the user is trying to enroll.
+
+ Returns:
+ AccessResponse, indicating whether the user can enroll.
+ """
+ enrollment_domain = courselike.enrollment_domain
+ # Courselike objects (e.g., course descriptors and CourseOverviews) have an attribute named `id`
+ # which actually points to a CourseKey. Sigh.
+ course_key = courselike.id
+
+ # If using a registration method to restrict enrollment (e.g., Shibboleth)
+ if settings.FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and enrollment_domain:
+ if user is not None and user.is_authenticated() and \
+ ExternalAuthMap.objects.filter(user=user, external_domain=enrollment_domain):
+ debug("Allow: external_auth of " + enrollment_domain)
+ reg_method_ok = True
+ else:
+ reg_method_ok = False
+ else:
+ reg_method_ok = True
+
+ # If the user appears in CourseEnrollmentAllowed paired with the given course key,
+ # they may enroll. Note that as dictated by the legacy database schema, the filter
+ # call includes a `course_id` kwarg which requires a CourseKey.
+ if user is not None and user.is_authenticated():
+ if CourseEnrollmentAllowed.objects.filter(email=user.email, course_id=course_key):
+ return ACCESS_GRANTED
+
+ if _has_staff_access_to_descriptor(user, courselike, course_key):
+ return ACCESS_GRANTED
+
+ if courselike.invitation_only:
+ debug("Deny: invitation only")
+ return ACCESS_DENIED
+
+ now = datetime.now(UTC())
+ enrollment_start = courselike.enrollment_start or datetime.min.replace(tzinfo=pytz.UTC)
+ enrollment_end = courselike.enrollment_end or datetime.max.replace(tzinfo=pytz.UTC)
+ if reg_method_ok and enrollment_start < now < enrollment_end:
+ debug("Allow: in enrollment period")
+ return ACCESS_GRANTED
+
+ return ACCESS_DENIED
+
+
def _has_access_course_desc(user, action, course):
"""
Check if user has access to a course descriptor.
@@ -271,54 +323,7 @@ def _has_access_course_desc(user, action, course):
return _has_access_descriptor(user, 'load', course, course.id)
def can_enroll():
- """
- First check if restriction of enrollment by login method is enabled, both
- globally and by the course.
- If it is, then the user must pass the criterion set by the course, e.g. that ExternalAuthMap
- was set by 'shib:https://idp.stanford.edu/", in addition to requirements below.
- Rest of requirements:
- (CourseEnrollmentAllowed always overrides)
- or
- (staff can always enroll)
- or
- Enrollment can only happen in the course enrollment period, if one exists, and
- course is not invitation only.
- """
-
- # if using registration method to restrict (say shibboleth)
- if settings.FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain:
- if user is not None and user.is_authenticated() and \
- ExternalAuthMap.objects.filter(user=user, external_domain=course.enrollment_domain):
- debug("Allow: external_auth of " + course.enrollment_domain)
- reg_method_ok = True
- else:
- reg_method_ok = False
- else:
- reg_method_ok = True # if not using this access check, it's always OK.
-
- now = datetime.now(UTC())
- start = course.enrollment_start or datetime.min.replace(tzinfo=pytz.UTC)
- end = course.enrollment_end or datetime.max.replace(tzinfo=pytz.UTC)
-
- # if user is in CourseEnrollmentAllowed with right course key then can also enroll
- # (note that course.id actually points to a CourseKey)
- # (the filter call uses course_id= since that's the legacy database schema)
- # (sorry that it's confusing :( )
- if user is not None and user.is_authenticated() and CourseEnrollmentAllowed:
- if CourseEnrollmentAllowed.objects.filter(email=user.email, course_id=course.id):
- return ACCESS_GRANTED
-
- if _has_staff_access_to_descriptor(user, course, course.id):
- return ACCESS_GRANTED
-
- # Invitation_only doesn't apply to CourseEnrollmentAllowed or has_staff_access_access
- if course.invitation_only:
- debug("Deny: invitation only")
- return ACCESS_DENIED
-
- if reg_method_ok and start < now < end:
- debug("Allow: in enrollment period")
- return ACCESS_GRANTED
+ return _can_enroll_courselike(user, course)
def see_exists():
"""
@@ -412,6 +417,7 @@ def _can_load_course_overview(user, course_overview):
)
_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)
diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py
index 8d32023d5c7f663b04243de1346fdf34adf0ed80..109a77e32be5659f1bdf0f63bf62fa79d94c2e3b 100644
--- a/lms/djangoapps/courseware/tests/test_access.py
+++ b/lms/djangoapps/courseware/tests/test_access.py
@@ -515,6 +515,12 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase):
self.user_staff = UserFactory.create(is_staff=True)
self.user_anonymous = AnonymousUserFactory.create()
+ ENROLL_TEST_DATA = list(itertools.product(
+ ['user_normal', 'user_staff', 'user_anonymous'],
+ ['enroll'],
+ ['course_default', 'course_started', 'course_not_started', 'course_staff_only'],
+ ))
+
LOAD_TEST_DATA = list(itertools.product(
['user_normal', 'user_beta_tester', 'user_staff'],
['load'],
@@ -533,7 +539,7 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase):
['course_default', 'course_with_pre_requisite', 'course_with_pre_requisites'],
))
- @ddt.data(*(LOAD_TEST_DATA + LOAD_MOBILE_TEST_DATA + PREREQUISITES_TEST_DATA))
+ @ddt.data(*(ENROLL_TEST_DATA + LOAD_TEST_DATA + LOAD_MOBILE_TEST_DATA + PREREQUISITES_TEST_DATA))
@ddt.unpack
def test_course_overview_access(self, user_attr_name, action, course_attr_name):
"""
diff --git a/openedx/core/djangoapps/content/course_overviews/migrations/0004_auto__add_field_courseoverview_enrollment_start__add_field_courseoverv.py b/openedx/core/djangoapps/content/course_overviews/migrations/0004_auto__add_field_courseoverview_enrollment_start__add_field_courseoverv.py
new file mode 100644
index 0000000000000000000000000000000000000000..01a28b368949ef4e1caed6efc289ee556993b9b6
--- /dev/null
+++ b/openedx/core/djangoapps/content/course_overviews/migrations/0004_auto__add_field_courseoverview_enrollment_start__add_field_courseoverv.py
@@ -0,0 +1,94 @@
+# -*- 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):
+ # The default values for these new columns may not match the actual
+ # values of courses already present in the table. To ensure that the
+ # cached values are correct, we must clear the table before adding any
+ # new columns.
+ db.clear_table('course_overviews_courseoverview')
+
+ # Adding field 'CourseOverview.enrollment_start'
+ db.add_column('course_overviews_courseoverview', 'enrollment_start',
+ self.gf('django.db.models.fields.DateTimeField')(null=True),
+ keep_default=False)
+
+ # Adding field 'CourseOverview.enrollment_end'
+ db.add_column('course_overviews_courseoverview', 'enrollment_end',
+ self.gf('django.db.models.fields.DateTimeField')(null=True),
+ keep_default=False)
+
+ # Adding field 'CourseOverview.enrollment_domain'
+ db.add_column('course_overviews_courseoverview', 'enrollment_domain',
+ self.gf('django.db.models.fields.TextField')(null=True),
+ keep_default=False)
+
+ # Adding field 'CourseOverview.invitation_only'
+ db.add_column('course_overviews_courseoverview', 'invitation_only',
+ self.gf('django.db.models.fields.BooleanField')(default=False),
+ keep_default=False)
+
+ # Adding field 'CourseOverview.max_student_enrollments_allowed'
+ db.add_column('course_overviews_courseoverview', 'max_student_enrollments_allowed',
+ self.gf('django.db.models.fields.IntegerField')(null=True),
+ keep_default=False)
+
+
+ def backwards(self, orm):
+ # Deleting field 'CourseOverview.enrollment_start'
+ db.delete_column('course_overviews_courseoverview', 'enrollment_start')
+
+ # Deleting field 'CourseOverview.enrollment_end'
+ db.delete_column('course_overviews_courseoverview', 'enrollment_end')
+
+ # Deleting field 'CourseOverview.enrollment_domain'
+ db.delete_column('course_overviews_courseoverview', 'enrollment_domain')
+
+ # Deleting field 'CourseOverview.invitation_only'
+ db.delete_column('course_overviews_courseoverview', 'invitation_only')
+
+ # Deleting field 'CourseOverview.max_student_enrollments_allowed'
+ db.delete_column('course_overviews_courseoverview', 'max_student_enrollments_allowed')
+
+
+ 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_html_view_enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ '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', [], {}),
+ 'days_early_for_beta': ('django.db.models.fields.FloatField', [], {'null': 'True'}),
+ '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'}),
+ 'enrollment_domain': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+ 'enrollment_end': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+ 'enrollment_start': ('django.db.models.fields.DateTimeField', [], {'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'}),
+ 'invitation_only': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'lowest_passing_grade': ('django.db.models.fields.DecimalField', [], {'max_digits': '5', 'decimal_places': '2'}),
+ 'max_student_enrollments_allowed': ('django.db.models.fields.IntegerField', [], {'null': 'True'}),
+ '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/models.py b/openedx/core/djangoapps/content/course_overviews/models.py
index 4b8429f9cc7183d75a9ec4dffc7e4ba1f4aaca0e..517eda28a540d56587e50f54745d066491a4611c 100644
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/openedx/core/djangoapps/content/course_overviews/models.py
@@ -5,7 +5,7 @@ Declaration of CourseOverview model
import json
import django.db.models
-from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField
+from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField, IntegerField
from django.utils.translation import ugettext
from util.date_utils import strftime_localized
@@ -60,6 +60,13 @@ class CourseOverview(django.db.models.Model):
visible_to_staff_only = BooleanField()
_pre_requisite_courses_json = TextField() # JSON representation of list of CourseKey strings
+ # Enrollment details
+ enrollment_start = DateTimeField(null=True)
+ enrollment_end = DateTimeField(null=True)
+ enrollment_domain = TextField(null=True)
+ invitation_only = BooleanField(default=False)
+ max_student_enrollments_allowed = IntegerField(null=True)
+
@staticmethod
def _create_from_course(course):
"""
@@ -114,7 +121,13 @@ class CourseOverview(django.db.models.Model):
days_early_for_beta=course.days_early_for_beta,
mobile_available=course.mobile_available,
visible_to_staff_only=course.visible_to_staff_only,
- _pre_requisite_courses_json=json.dumps(course.pre_requisite_courses)
+ _pre_requisite_courses_json=json.dumps(course.pre_requisite_courses),
+
+ enrollment_start=course.enrollment_start,
+ enrollment_end=course.enrollment_end,
+ enrollment_domain=course.enrollment_domain,
+ invitation_only=course.invitation_only,
+ max_student_enrollments_allowed=course.max_student_enrollments_allowed,
)
@staticmethod
diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py
index 37bfcd268155b996847119b8a4f0356cc8956d4a..c2141847b12ff92f6067c24f82e3a97bda80f0f9 100644
--- a/openedx/core/djangoapps/content/course_overviews/signals.py
+++ b/openedx/core/djangoapps/content/course_overviews/signals.py
@@ -3,9 +3,8 @@ Signal handler for invalidating cached course overviews
"""
from django.dispatch.dispatcher import receiver
-from xmodule.modulestore.django import SignalHandler
-
from .models import CourseOverview
+from xmodule.modulestore.django import SignalHandler
@receiver(SignalHandler.course_published)
@@ -15,3 +14,12 @@ def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable
invalidates the corresponding CourseOverview cache entry if one exists.
"""
CourseOverview.objects.filter(id=course_key).delete()
+
+
+@receiver(SignalHandler.course_deleted)
+def _listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument
+ """
+ Catches the signal that a course has been deleted from 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
index 8a56bb51725948be2e6e422215b1d96f6d74cde4..25035b26061d3f96a9fdb96a1d463746b8d0dfb8 100644
--- a/openedx/core/djangoapps/content/course_overviews/tests.py
+++ b/openedx/core/djangoapps/content/course_overviews/tests.py
@@ -91,6 +91,9 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
'display_name_with_default',
'start_date_is_still_default',
'pre_requisite_courses',
+ 'enrollment_domain',
+ 'invitation_only',
+ 'max_student_enrollments_allowed',
]
for attribute_name in fields_to_test:
course_value = getattr(course, attribute_name)
@@ -125,24 +128,38 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
# 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),
- )]
+ 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),
+ ),
+ (
+ 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),
+ ),
+ (
+ 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),
+ ),
+ ]
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)
@@ -211,7 +228,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
@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
+ Tests that when a course is published or deleted, the corresponding
course_overview is removed from the cache.
Arguments:
@@ -236,6 +253,11 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
course_overview_2 = CourseOverview.get_from_id(course.id)
self.assertFalse(course_overview_2.mobile_available)
+ # Verify that when the course is deleted, the corresponding CourseOverview is deleted as well.
+ with self.assertRaises(CourseOverview.DoesNotExist):
+ self.store.delete_course(course.id, ModuleStoreEnum.UserID.test)
+ CourseOverview.get_from_id(course.id)
+
@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):