Commit 845e1d62 by Adam

Merge pull request #8998 from edx/merge-release-into-master

Merge release into master
parents 9e634a17 e5158423
...@@ -6,13 +6,12 @@ from io import BytesIO ...@@ -6,13 +6,12 @@ from io import BytesIO
from pytz import UTC from pytz import UTC
from PIL import Image from PIL import Image
import json import json
from mock import patch
from django.conf import settings from django.conf import settings
from contentstore.tests.utils import CourseTestCase from contentstore.tests.utils import CourseTestCase
from contentstore.views import assets from contentstore.views import assets
from contentstore.utils import reverse_course_url from contentstore.utils import reverse_course_url
from xmodule.assetstore.assetmgr import AssetMetadataFoundTemporary
from xmodule.assetstore import AssetMetadata from xmodule.assetstore import AssetMetadata
from xmodule.contentstore.content import StaticContent from xmodule.contentstore.content import StaticContent
from xmodule.contentstore.django import contentstore from xmodule.contentstore.django import contentstore
...@@ -331,23 +330,13 @@ class DownloadTestCase(AssetsTestCase): ...@@ -331,23 +330,13 @@ class DownloadTestCase(AssetsTestCase):
resp = self.client.get(url, HTTP_ACCEPT='text/html') resp = self.client.get(url, HTTP_ACCEPT='text/html')
self.assertEquals(resp.status_code, 404) self.assertEquals(resp.status_code, 404)
def test_metadata_found_in_modulestore(self): @patch('xmodule.modulestore.mixed.MixedModuleStore.find_asset_metadata')
# Insert asset metadata into the modulestore (with no accompanying asset). def test_pickling_calls(self, patched_find_asset_metadata):
asset_key = self.course.id.make_asset_key(AssetMetadata.GENERAL_ASSET_TYPE, 'pic1.jpg') """ Tests if assets are not calling find_asset_metadata
asset_md = AssetMetadata(asset_key, { """
'internal_name': 'EKMND332DDBK', patched_find_asset_metadata.return_value = None
'basename': 'pix/archive', self.client.get(self.uploaded_url, HTTP_ACCEPT='text/html')
'locked': False, self.assertFalse(patched_find_asset_metadata.called)
'curr_version': '14',
'prev_version': '13'
})
modulestore().save_asset_metadata(asset_md, 15)
# Get the asset metadata and have it be found in the modulestore.
# Currently, no asset metadata should be found in the modulestore. The code is not yet storing it there.
# If asset metadata *is* found there, an exception is raised. This test ensures the exception is indeed raised.
# THIS IS TEMPORARY. Soon, asset metadata *will* be stored in the modulestore.
with self.assertRaises((AssetMetadataFoundTemporary, NameError)):
self.client.get(unicode(asset_key), HTTP_ACCEPT='text/html')
class AssetToJsonTestCase(AssetsTestCase): class AssetToJsonTestCase(AssetsTestCase):
......
"""
A cache that is cleared after every request.
This module requires that :class:`request_cache.middleware.RequestCache`
is installed in order to clear the cache after each request.
"""
from request_cache import middleware
def get_cache(name):
"""
Return the request cache named ``name``.
Arguments:
name (str): The name of the request cache to load
Returns: dict
"""
return middleware.RequestCache.get_request_cache(name)
def get_request():
"""
Return the current request.
"""
return middleware.RequestCache.get_current_request()
import threading import threading
_request_cache_threadlocal = threading.local()
_request_cache_threadlocal.data = {} class _RequestCache(threading.local):
_request_cache_threadlocal.request = None """
A thread-local for storing the per-request cache.
"""
def __init__(self):
super(_RequestCache, self).__init__()
self.data = {}
self.request = None
REQUEST_CACHE = _RequestCache()
class RequestCache(object): class RequestCache(object):
@classmethod @classmethod
def get_request_cache(cls): def get_request_cache(cls, name=None):
return _request_cache_threadlocal """
This method is deprecated. Please use :func:`request_cache.get_cache`.
"""
if name is None:
return REQUEST_CACHE
else:
return REQUEST_CACHE.data.setdefault(name, {})
@classmethod @classmethod
def get_current_request(cls): def get_current_request(cls):
""" """
Get a reference to the HttpRequest object, if we are presently This method is deprecated. Please use :func:`request_cache.get_request`.
servicing one.
""" """
return _request_cache_threadlocal.request return REQUEST_CACHE.request
@classmethod @classmethod
def clear_request_cache(cls): def clear_request_cache(cls):
""" """
Empty the request cache. Empty the request cache.
""" """
_request_cache_threadlocal.data = {} REQUEST_CACHE.data = {}
_request_cache_threadlocal.request = None REQUEST_CACHE.request = None
def process_request(self, request): def process_request(self, request):
self.clear_request_cache() self.clear_request_cache()
_request_cache_threadlocal.request = request REQUEST_CACHE.request = request
return None return None
def process_response(self, request, response): def process_response(self, request, response):
......
...@@ -9,11 +9,12 @@ Handles: ...@@ -9,11 +9,12 @@ Handles:
Phase 1: Checks to see if an asset's metadata can be found in the course's modulestore. Phase 1: Checks to see if an asset's metadata can be found in the course's modulestore.
If not found, fails over to access the asset from the contentstore. If not found, fails over to access the asset from the contentstore.
At first, the asset metadata will never be found, since saving isn't implemented yet. At first, the asset metadata will never be found, since saving isn't implemented yet.
Note: Hotfix (PLAT-734) No asset calls find_asset_metadata, and directly accesses from contentstore.
""" """
from contracts import contract, new_contract from contracts import contract, new_contract
from opaque_keys.edx.keys import AssetKey from opaque_keys.edx.keys import AssetKey
from xmodule.modulestore.django import modulestore
from xmodule.contentstore.django import contentstore from xmodule.contentstore.django import contentstore
...@@ -49,14 +50,11 @@ class AssetManager(object): ...@@ -49,14 +50,11 @@ class AssetManager(object):
@contract(asset_key='AssetKey', throw_on_not_found='bool', as_stream='bool') @contract(asset_key='AssetKey', throw_on_not_found='bool', as_stream='bool')
def find(asset_key, throw_on_not_found=True, as_stream=False): def find(asset_key, throw_on_not_found=True, as_stream=False):
""" """
Finds a course asset either in the assetstore -or- in the deprecated contentstore. Finds course asset in the deprecated contentstore.
This method was previously searching for the course asset in the assetstore first, then in the deprecated
contentstore. However, the asset was never found in the assetstore since an asset's metadata is
not yet stored there.(removed calls to modulestore().find_asset_metadata(asset_key))
The assetstore search was removed due to performance issues caused by each call unpickling the pickled and
compressed course structure from the structure cache.
""" """
content_md = modulestore().find_asset_metadata(asset_key)
# If found, raise an exception.
if content_md:
# For now, no asset metadata should be found in the modulestore.
raise AssetMetadataFoundTemporary()
else:
# If not found, load the asset via the contentstore.
return contentstore().find(asset_key, throw_on_not_found, as_stream) return contentstore().find(asset_key, throw_on_not_found, as_stream)
...@@ -65,9 +65,13 @@ class DiscussionModule(DiscussionFields, XModule): ...@@ -65,9 +65,13 @@ class DiscussionModule(DiscussionFields, XModule):
def get_course(self): def get_course(self):
""" """
Return course by course id. Return the CourseDescriptor at the root of the tree we're in.
""" """
return self.descriptor.runtime.modulestore.get_course(self.course_id) block = self
while block.parent:
block = block.get_parent()
return block
class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawDescriptor): class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawDescriptor):
......
...@@ -7,6 +7,8 @@ import logging ...@@ -7,6 +7,8 @@ import logging
from django.db import transaction, IntegrityError from django.db import transaction, IntegrityError
import request_cache
from courseware.field_overrides import FieldOverrideProvider # pylint: disable=import-error from courseware.field_overrides import FieldOverrideProvider # pylint: disable=import-error
from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.keys import CourseKey, UsageKey
from ccx_keys.locator import CCXLocator, CCXBlockUsageLocator from ccx_keys.locator import CCXLocator, CCXBlockUsageLocator
...@@ -69,7 +71,11 @@ def get_current_ccx(course_key): ...@@ -69,7 +71,11 @@ def get_current_ccx(course_key):
if not isinstance(course_key, CCXLocator): if not isinstance(course_key, CCXLocator):
return None return None
return CustomCourseForEdX.objects.get(pk=course_key.ccx) ccx_cache = request_cache.get_cache('ccx')
if course_key not in ccx_cache:
ccx_cache[course_key] = CustomCourseForEdX.objects.get(pk=course_key.ccx)
return ccx_cache[course_key]
def get_override_for_ccx(ccx, block, name, default=None): def get_override_for_ccx(ccx, block, name, default=None):
...@@ -78,35 +84,40 @@ def get_override_for_ccx(ccx, block, name, default=None): ...@@ -78,35 +84,40 @@ def get_override_for_ccx(ccx, block, name, default=None):
specify the block and the name of the field. If the field is not specify the block and the name of the field. If the field is not
overridden for the given ccx, returns `default`. overridden for the given ccx, returns `default`.
""" """
if not hasattr(block, '_ccx_overrides'): overrides = _get_overrides_for_ccx(ccx)
block._ccx_overrides = {} # pylint: disable=protected-access
overrides = block._ccx_overrides.get(ccx.id) # pylint: disable=protected-access if isinstance(block.location, CCXBlockUsageLocator):
if overrides is None: non_ccx_key = block.location.to_block_locator()
overrides = _get_overrides_for_ccx(ccx, block) else:
block._ccx_overrides[ccx.id] = overrides # pylint: disable=protected-access non_ccx_key = block.location
return overrides.get(name, default)
block_overrides = overrides.get(non_ccx_key, {})
if name in block_overrides:
return block.fields[name].from_json(block_overrides[name])
else:
return default
def _get_overrides_for_ccx(ccx, block): def _get_overrides_for_ccx(ccx):
""" """
Returns a dictionary mapping field name to overriden value for any Returns a dictionary mapping field name to overriden value for any
overrides set on this block for this CCX. overrides set on this block for this CCX.
""" """
overrides_cache = request_cache.get_cache('ccx-overrides')
if ccx not in overrides_cache:
overrides = {} overrides = {}
# block as passed in may have a location specific to a CCX, we must strip
# that for this query
location = block.location
if isinstance(block.location, CCXBlockUsageLocator):
location = block.location.to_block_locator()
query = CcxFieldOverride.objects.filter( query = CcxFieldOverride.objects.filter(
ccx=ccx, ccx=ccx,
location=location
) )
for override in query: for override in query:
field = block.fields[override.field] block_overrides = overrides.setdefault(override.location, {})
value = field.from_json(json.loads(override.value)) block_overrides[override.field] = json.loads(override.value)
overrides[override.field] = value
return overrides overrides_cache[ccx] = overrides
return overrides_cache[ccx]
@transaction.commit_on_success @transaction.commit_on_success
...@@ -117,23 +128,25 @@ def override_field_for_ccx(ccx, block, name, value): ...@@ -117,23 +128,25 @@ def override_field_for_ccx(ccx, block, name, value):
value to set for the given field. value to set for the given field.
""" """
field = block.fields[name] field = block.fields[name]
value = json.dumps(field.to_json(value)) value_json = field.to_json(value)
serialized_value = json.dumps(value_json)
try: try:
override = CcxFieldOverride.objects.create( override = CcxFieldOverride.objects.create(
ccx=ccx, ccx=ccx,
location=block.location, location=block.location,
field=name, field=name,
value=value) value=serialized_value
)
except IntegrityError: except IntegrityError:
transaction.commit() transaction.commit()
override = CcxFieldOverride.objects.get( override = CcxFieldOverride.objects.get(
ccx=ccx, ccx=ccx,
location=block.location, location=block.location,
field=name) field=name
override.value = value )
override.value = serialized_value
override.save() override.save()
if hasattr(block, '_ccx_overrides'): _get_overrides_for_ccx(ccx).setdefault(block.location, {})[name] = value_json
del block._ccx_overrides[ccx.id] # pylint: disable=protected-access
def clear_override_for_ccx(ccx, block, name): def clear_override_for_ccx(ccx, block, name):
...@@ -149,8 +162,7 @@ def clear_override_for_ccx(ccx, block, name): ...@@ -149,8 +162,7 @@ def clear_override_for_ccx(ccx, block, name):
location=block.location, location=block.location,
field=name).delete() field=name).delete()
if hasattr(block, '_ccx_overrides'): _get_overrides_for_ccx(ccx).setdefault(block.location, {}).pop(name)
del block._ccx_overrides[ccx.id] # pylint: disable=protected-access
except CcxFieldOverride.DoesNotExist: except CcxFieldOverride.DoesNotExist:
pass pass
""" """
Dummy factories for tests Dummy factories for tests
""" """
from factory import SubFactory
from factory.django import DjangoModelFactory from factory.django import DjangoModelFactory
from student.tests.factories import UserFactory
from ccx.models import CustomCourseForEdX # pylint: disable=import-error from ccx.models import CustomCourseForEdX # pylint: disable=import-error
from ccx.models import CcxMembership # pylint: disable=import-error from ccx.models import CcxMembership # pylint: disable=import-error
from ccx.models import CcxFutureMembership # pylint: disable=import-error from ccx.models import CcxFutureMembership # pylint: disable=import-error
...@@ -11,6 +13,7 @@ class CcxFactory(DjangoModelFactory): # pylint: disable=missing-docstring ...@@ -11,6 +13,7 @@ class CcxFactory(DjangoModelFactory): # pylint: disable=missing-docstring
FACTORY_FOR = CustomCourseForEdX FACTORY_FOR = CustomCourseForEdX
display_name = "Test CCX" display_name = "Test CCX"
id = None # pylint: disable=redefined-builtin, invalid-name id = None # pylint: disable=redefined-builtin, invalid-name
coach = SubFactory(UserFactory)
class CcxMembershipFactory(DjangoModelFactory): # pylint: disable=missing-docstring class CcxMembershipFactory(DjangoModelFactory): # pylint: disable=missing-docstring
......
...@@ -5,6 +5,7 @@ Performance tests for field overrides. ...@@ -5,6 +5,7 @@ Performance tests for field overrides.
import ddt import ddt
import itertools import itertools
import mock import mock
from nose.plugins.skip import SkipTest
from courseware.views import progress # pylint: disable=import-error from courseware.views import progress # pylint: disable=import-error
from courseware.field_overrides import OverrideFieldData from courseware.field_overrides import OverrideFieldData
...@@ -25,6 +26,8 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \ ...@@ -25,6 +26,8 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \
TEST_DATA_SPLIT_MODULESTORE, TEST_DATA_MONGO_MODULESTORE TEST_DATA_SPLIT_MODULESTORE, TEST_DATA_MONGO_MODULESTORE
from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, check_sum_of_calls from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, check_sum_of_calls
from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin
from ccx_keys.locator import CCXLocator
from ccx.tests.factories import CcxFactory, CcxMembershipFactory
@attr('shard_1') @attr('shard_1')
...@@ -58,6 +61,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -58,6 +61,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
self.request = self.request_factory.get("foo") self.request = self.request_factory.get("foo")
self.request.user = self.student self.request.user = self.student
self.course = None self.course = None
self.ccx = None
MakoMiddleware().process_request(self.request) MakoMiddleware().process_request(self.request)
...@@ -113,17 +117,42 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -113,17 +117,42 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
self.course.id self.course.id
) )
def grade_course(self, course): if enable_ccx:
self.ccx = CcxFactory.create()
CcxMembershipFactory.create(
student=self.student,
ccx=self.ccx
)
def grade_course(self, course, view_as_ccx):
""" """
Renders the progress page for the given course. Renders the progress page for the given course.
""" """
course_key = course.id
if view_as_ccx:
course_key = CCXLocator.from_course_locator(course_key, self.ccx.id)
return progress( return progress(
self.request, self.request,
course_id=course.id.to_deprecated_string(), course_id=unicode(course_key),
student_id=self.student.id student_id=self.student.id
) )
def instrument_course_progress_render(self, course_width, enable_ccx, queries, reads, xblocks): def assertMongoCallCount(self, calls):
"""
Assert that mongodb is queried ``calls`` times in the surrounded
context.
"""
return check_mongo_calls(calls)
def assertXBlockInstantiations(self, instantiations):
"""
Assert that exactly ``instantiations`` XBlocks are instantiated in
the surrounded context.
"""
return check_sum_of_calls(XBlock, ['__init__'], instantiations, instantiations, include_arguments=False)
def instrument_course_progress_render(self, course_width, enable_ccx, view_as_ccx, queries, reads, xblocks):
""" """
Renders the progress page, instrumenting Mongo reads and SQL queries. Renders the progress page, instrumenting Mongo reads and SQL queries.
""" """
...@@ -146,16 +175,16 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -146,16 +175,16 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
OverrideFieldData.provider_classes = None OverrideFieldData.provider_classes = None
with self.assertNumQueries(queries): with self.assertNumQueries(queries):
with check_mongo_calls(reads): with self.assertMongoCallCount(reads):
with check_sum_of_calls(XBlock, ['__init__'], xblocks, xblocks, include_arguments=False): with self.assertXBlockInstantiations(xblocks):
self.grade_course(self.course) self.grade_course(self.course, view_as_ccx)
@ddt.data(*itertools.product(('no_overrides', 'ccx'), range(1, 4), (True, False))) @ddt.data(*itertools.product(('no_overrides', 'ccx'), range(1, 4), (True, False), (True, False)))
@ddt.unpack @ddt.unpack
@override_settings( @override_settings(
FIELD_OVERRIDE_PROVIDERS=(), FIELD_OVERRIDE_PROVIDERS=(),
) )
def test_field_overrides(self, overrides, course_width, enable_ccx): def test_field_overrides(self, overrides, course_width, enable_ccx, view_as_ccx):
""" """
Test without any field overrides. Test without any field overrides.
""" """
...@@ -163,9 +192,18 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -163,9 +192,18 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
'no_overrides': (), 'no_overrides': (),
'ccx': ('ccx.overrides.CustomCoursesForEdxOverrideProvider',) 'ccx': ('ccx.overrides.CustomCoursesForEdxOverrideProvider',)
} }
if overrides == 'no_overrides' and view_as_ccx:
raise SkipTest("Can't view a ccx course if field overrides are disabled.")
if not enable_ccx and view_as_ccx:
raise SkipTest("Can't view a ccx course if ccx is disabled on the course")
if self.MODULESTORE == TEST_DATA_MONGO_MODULESTORE and view_as_ccx:
raise SkipTest("Can't use a MongoModulestore test as a CCX course")
with self.settings(FIELD_OVERRIDE_PROVIDERS=providers[overrides]): with self.settings(FIELD_OVERRIDE_PROVIDERS=providers[overrides]):
queries, reads, xblocks = self.TEST_DATA[(overrides, course_width, enable_ccx)] queries, reads, xblocks = self.TEST_DATA[(overrides, course_width, enable_ccx, view_as_ccx)]
self.instrument_course_progress_render(course_width, enable_ccx, queries, reads, xblocks) self.instrument_course_progress_render(course_width, enable_ccx, view_as_ccx, queries, reads, xblocks)
class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
...@@ -176,19 +214,25 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): ...@@ -176,19 +214,25 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
__test__ = True __test__ = True
TEST_DATA = { TEST_DATA = {
# (providers, course_width, enable_ccx): # of sql queries, # of mongo queries, # of xblocks # (providers, course_width, enable_ccx, view_as_ccx): # of sql queries, # of mongo queries, # of xblocks
('no_overrides', 1, True): (23, 7, 14), ('no_overrides', 1, True, False): (23, 7, 14),
('no_overrides', 2, True): (68, 7, 85), ('no_overrides', 2, True, False): (68, 7, 85),
('no_overrides', 3, True): (263, 7, 336), ('no_overrides', 3, True, False): (263, 7, 336),
('ccx', 1, True): (23, 7, 14), ('ccx', 1, True, False): (23, 7, 14),
('ccx', 2, True): (68, 7, 85), ('ccx', 2, True, False): (68, 7, 85),
('ccx', 3, True): (263, 7, 336), ('ccx', 3, True, False): (263, 7, 336),
('no_overrides', 1, False): (23, 7, 14), ('ccx', 1, True, True): (23, 7, 14),
('no_overrides', 2, False): (68, 7, 85), ('ccx', 2, True, True): (68, 7, 85),
('no_overrides', 3, False): (263, 7, 336), ('ccx', 3, True, True): (263, 7, 336),
('ccx', 1, False): (23, 7, 14), ('no_overrides', 1, False, False): (23, 7, 14),
('ccx', 2, False): (68, 7, 85), ('no_overrides', 2, False, False): (68, 7, 85),
('ccx', 3, False): (263, 7, 336), ('no_overrides', 3, False, False): (263, 7, 336),
('ccx', 1, False, False): (23, 7, 14),
('ccx', 2, False, False): (68, 7, 85),
('ccx', 3, False, False): (263, 7, 336),
('ccx', 1, False, True): (23, 7, 14),
('ccx', 2, False, True): (68, 7, 85),
('ccx', 3, False, True): (263, 7, 336),
} }
...@@ -200,16 +244,22 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): ...@@ -200,16 +244,22 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True __test__ = True
TEST_DATA = { TEST_DATA = {
('no_overrides', 1, True): (23, 4, 9), ('no_overrides', 1, True, False): (23, 4, 9),
('no_overrides', 2, True): (68, 19, 54), ('no_overrides', 2, True, False): (68, 19, 54),
('no_overrides', 3, True): (263, 84, 215), ('no_overrides', 3, True, False): (263, 84, 215),
('ccx', 1, True): (23, 4, 9), ('ccx', 1, True, False): (23, 4, 9),
('ccx', 2, True): (68, 19, 54), ('ccx', 2, True, False): (68, 19, 54),
('ccx', 3, True): (263, 84, 215), ('ccx', 3, True, False): (263, 84, 215),
('no_overrides', 1, False): (23, 4, 9), ('ccx', 1, True, True): (25, 4, 13),
('no_overrides', 2, False): (68, 19, 54), ('ccx', 2, True, True): (70, 19, 84),
('no_overrides', 3, False): (263, 84, 215), ('ccx', 3, True, True): (265, 84, 335),
('ccx', 1, False): (23, 4, 9), ('no_overrides', 1, False, False): (23, 4, 9),
('ccx', 2, False): (68, 19, 54), ('no_overrides', 2, False, False): (68, 19, 54),
('ccx', 3, False): (263, 84, 215), ('no_overrides', 3, False, False): (263, 84, 215),
('ccx', 1, False, False): (23, 4, 9),
('ccx', 2, False, False): (68, 19, 54),
('ccx', 3, False, False): (263, 84, 215),
('ccx', 1, False, True): (23, 4, 9),
('ccx', 2, False, True): (68, 19, 54),
('ccx', 3, False, True): (263, 84, 215),
} }
...@@ -9,6 +9,7 @@ from nose.plugins.attrib import attr ...@@ -9,6 +9,7 @@ from nose.plugins.attrib import attr
from courseware.field_overrides import OverrideFieldData # pylint: disable=import-error from courseware.field_overrides import OverrideFieldData # pylint: disable=import-error
from django.test.utils import override_settings from django.test.utils import override_settings
from request_cache.middleware import RequestCache
from student.tests.factories import AdminFactory # pylint: disable=import-error from student.tests.factories import AdminFactory # pylint: disable=import-error
from xmodule.modulestore.tests.django_utils import ( from xmodule.modulestore.tests.django_utils import (
ModuleStoreTestCase, ModuleStoreTestCase,
...@@ -66,6 +67,8 @@ class TestFieldOverrides(ModuleStoreTestCase): ...@@ -66,6 +67,8 @@ class TestFieldOverrides(ModuleStoreTestCase):
get_ccx.return_value = ccx get_ccx.return_value = ccx
self.addCleanup(patch.stop) self.addCleanup(patch.stop)
self.addCleanup(RequestCache.clear_request_cache)
# Apparently the test harness doesn't use LmsFieldStorage, and I'm not # Apparently the test harness doesn't use LmsFieldStorage, and I'm not
# sure if there's a way to poke the test harness to do so. So, we'll # sure if there's a way to poke the test harness to do so. So, we'll
# just inject the override field storage in this brute force manner. # just inject the override field storage in this brute force manner.
...@@ -97,7 +100,7 @@ class TestFieldOverrides(ModuleStoreTestCase): ...@@ -97,7 +100,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
""" """
ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC) ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
chapter = self.ccx.course.get_children()[0] chapter = self.ccx.course.get_children()[0]
with self.assertNumQueries(4): with self.assertNumQueries(3):
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start) override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
dummy = chapter.start dummy = chapter.start
...@@ -107,7 +110,7 @@ class TestFieldOverrides(ModuleStoreTestCase): ...@@ -107,7 +110,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
""" """
ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC) ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
chapter = self.ccx.course.get_children()[0] chapter = self.ccx.course.get_children()[0]
with self.assertNumQueries(4): with self.assertNumQueries(3):
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start) override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
dummy1 = chapter.start dummy1 = chapter.start
dummy2 = chapter.start dummy2 = chapter.start
......
...@@ -557,9 +557,9 @@ class TestGetMembershipTriplets(ModuleStoreTestCase): ...@@ -557,9 +557,9 @@ class TestGetMembershipTriplets(ModuleStoreTestCase):
self.make_ccx_membership() self.make_ccx_membership()
triplets = self.call_fut() triplets = self.call_fut()
self.assertEqual(len(triplets), 1) self.assertEqual(len(triplets), 1)
ccx, membership, course = triplets[0] ccx, membership, course_overview = triplets[0]
self.assertEqual(ccx.id, self.ccx.id) self.assertEqual(ccx.id, self.ccx.id)
self.assertEqual(unicode(course.id), unicode(self.course.id)) self.assertEqual(unicode(course_overview.id), unicode(self.course.id))
self.assertEqual(membership.student, self.user) self.assertEqual(membership.student, self.user)
def test_has_membership_org_filtered(self): def test_has_membership_org_filtered(self):
......
...@@ -16,9 +16,12 @@ from courseware.tests.factories import StudentModuleFactory # pylint: disable=i ...@@ -16,9 +16,12 @@ from courseware.tests.factories import StudentModuleFactory # pylint: disable=i
from courseware.tests.helpers import LoginEnrollmentTestCase # pylint: disable=import-error from courseware.tests.helpers import LoginEnrollmentTestCase # pylint: disable=import-error
from courseware.tabs import get_course_tab_list from courseware.tabs import get_course_tab_list
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.utils.timezone import UTC
from django.test.utils import override_settings from django.test.utils import override_settings
from django.test import RequestFactory from django.test import RequestFactory
from edxmako.shortcuts import render_to_response # pylint: disable=import-error from edxmako.shortcuts import render_to_response # pylint: disable=import-error
from student.models import CourseEnrollment
from request_cache.middleware import RequestCache
from student.roles import CourseCcxCoachRole # pylint: disable=import-error from student.roles import CourseCcxCoachRole # pylint: disable=import-error
from student.tests.factories import ( # pylint: disable=import-error from student.tests.factories import ( # pylint: disable=import-error
AdminFactory, AdminFactory,
...@@ -27,6 +30,7 @@ from student.tests.factories import ( # pylint: disable=import-error ...@@ -27,6 +30,7 @@ from student.tests.factories import ( # pylint: disable=import-error
) )
from xmodule.x_module import XModuleMixin from xmodule.x_module import XModuleMixin
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import ( from xmodule.modulestore.tests.django_utils import (
ModuleStoreTestCase, ModuleStoreTestCase,
TEST_DATA_SPLIT_MODULESTORE) TEST_DATA_SPLIT_MODULESTORE)
...@@ -500,26 +504,6 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -500,26 +504,6 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
role.add_users(coach) role.add_users(coach)
ccx = CcxFactory(course_id=course.id, coach=self.coach) ccx = CcxFactory(course_id=course.id, coach=self.coach)
# Apparently the test harness doesn't use LmsFieldStorage, and I'm not
# sure if there's a way to poke the test harness to do so. So, we'll
# just inject the override field storage in this brute force manner.
OverrideFieldData.provider_classes = None
# pylint: disable=protected-access
for block in iter_blocks(course):
block._field_data = OverrideFieldData.wrap(coach, course, block._field_data)
new_cache = {'tabs': [], 'discussion_topics': []}
if 'grading_policy' in block._field_data_cache:
new_cache['grading_policy'] = block._field_data_cache['grading_policy']
block._field_data_cache = new_cache
def cleanup_provider_classes():
"""
After everything is done, clean up by un-doing the change to the
OverrideFieldData object that is done during the wrap method.
"""
OverrideFieldData.provider_classes = None
self.addCleanup(cleanup_provider_classes)
# override course grading policy and make last section invisible to students # override course grading policy and make last section invisible to students
override_field_for_ccx(ccx, course, 'grading_policy', { override_field_for_ccx(ccx, course, 'grading_policy', {
'GRADER': [ 'GRADER': [
...@@ -558,9 +542,13 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -558,9 +542,13 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.client.login(username=coach.username, password="test") self.client.login(username=coach.username, password="test")
self.addCleanup(RequestCache.clear_request_cache)
@patch('ccx.views.render_to_response', intercept_renderer) @patch('ccx.views.render_to_response', intercept_renderer)
def test_gradebook(self): def test_gradebook(self):
self.course.enable_ccx = True self.course.enable_ccx = True
RequestCache.clear_request_cache()
url = reverse( url = reverse(
'ccx_gradebook', 'ccx_gradebook',
kwargs={'course_id': self.ccx_key} kwargs={'course_id': self.ccx_key}
...@@ -577,6 +565,8 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -577,6 +565,8 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
def test_grades_csv(self): def test_grades_csv(self):
self.course.enable_ccx = True self.course.enable_ccx = True
RequestCache.clear_request_cache()
url = reverse( url = reverse(
'ccx_grades_csv', 'ccx_grades_csv',
kwargs={'course_id': self.ccx_key} kwargs={'course_id': self.ccx_key}
...@@ -588,11 +578,11 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -588,11 +578,11 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
response.content.strip().split('\n') response.content.strip().split('\n')
) )
data = dict(zip(headers, row)) data = dict(zip(headers, row))
self.assertTrue('HW 04' not in data)
self.assertEqual(data['HW 01'], '0.75') self.assertEqual(data['HW 01'], '0.75')
self.assertEqual(data['HW 02'], '0.5') self.assertEqual(data['HW 02'], '0.5')
self.assertEqual(data['HW 03'], '0.25') self.assertEqual(data['HW 03'], '0.25')
self.assertEqual(data['HW Avg'], '0.5') self.assertEqual(data['HW Avg'], '0.5')
self.assertTrue('HW 04' not in data)
@patch('courseware.views.render_to_response', intercept_renderer) @patch('courseware.views.render_to_response', intercept_renderer)
def test_student_progress(self): def test_student_progress(self):
...@@ -655,6 +645,43 @@ class CCXCoachTabTestCase(ModuleStoreTestCase): ...@@ -655,6 +645,43 @@ class CCXCoachTabTestCase(ModuleStoreTestCase):
) )
class TestStudentDashboardWithCCX(ModuleStoreTestCase):
"""
Test to ensure that the student dashboard works for users enrolled in CCX
courses.
"""
def setUp(self):
"""
Set up courses and enrollments.
"""
super(TestStudentDashboardWithCCX, self).setUp()
# Create a Draft Mongo and a Split Mongo course and enroll a student user in them.
self.student_password = "foobar"
self.student = UserFactory.create(username="test", password=self.student_password, is_staff=False)
self.draft_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.mongo)
self.split_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split)
CourseEnrollment.enroll(self.student, self.draft_course.id)
CourseEnrollment.enroll(self.student, self.split_course.id)
# Create a CCX coach.
self.coach = AdminFactory.create()
role = CourseCcxCoachRole(self.split_course.id)
role.add_users(self.coach)
# Create a CCX course and enroll the user in it.
self.ccx = CcxFactory(course_id=self.split_course.id, coach=self.coach)
last_week = datetime.datetime.now(UTC()) - datetime.timedelta(days=7)
override_field_for_ccx(self.ccx, self.split_course, 'start', last_week) # Required by self.ccx.has_started().
CcxMembershipFactory(ccx=self.ccx, student=self.student, active=True)
def test_load_student_dashboard(self):
self.client.login(username=self.student.username, password=self.student_password)
response = self.client.get(reverse('dashboard'))
self.assertEqual(response.status_code, 200)
def flatten(seq): def flatten(seq):
""" """
For [[1, 2], [3, 4]] returns [1, 2, 3, 4]. Does not recurse. For [[1, 2], [3, 4]] returns [1, 2, 3, 4]. Does not recurse.
......
...@@ -10,6 +10,7 @@ from django.core.urlresolvers import reverse ...@@ -10,6 +10,7 @@ from django.core.urlresolvers import reverse
from django.core.mail import send_mail from django.core.mail import send_mail
from edxmako.shortcuts import render_to_string # pylint: disable=import-error from edxmako.shortcuts import render_to_string # pylint: disable=import-error
from microsite_configuration import microsite # pylint: disable=import-error from microsite_configuration import microsite # pylint: disable=import-error
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.error_module import ErrorDescriptor from xmodule.error_module import ErrorDescriptor
from ccx_keys.locator import CCXLocator from ccx_keys.locator import CCXLocator
...@@ -240,29 +241,41 @@ def send_mail_to_student(student, param_dict): ...@@ -240,29 +241,41 @@ def send_mail_to_student(student, param_dict):
def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set): def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set):
""" """
Get the relevant set of (CustomCourseForEdX, CcxMembership, Course) Get the relevant set of (CustomCourseForEdX, CcxMembership, CourseOverview)
triplets to be displayed on a student's dashboard. triplets to be displayed on a student's dashboard.
""" """
error_message_format = "User {0} enrolled in {2} course {1}"
# only active memberships for now # only active memberships for now
for membership in CcxMembership.memberships_for_user(user): for membership in CcxMembership.memberships_for_user(user):
ccx = membership.ccx ccx = membership.ccx
store = modulestore()
with store.bulk_operations(ccx.course_id): try:
course = store.get_course(ccx.course_id) course_overview = CourseOverview.get_from_id(ccx.course_id)
if course and not isinstance(course, ErrorDescriptor): except CourseOverview.DoesNotExist:
log.error(error_message_format.format( # pylint: disable=logging-format-interpolation
user.username, ccx.course_id, "non-existent"
))
continue
except IOError:
log.error(error_message_format.format( # pylint: disable=logging-format-interpolation
user.username, ccx.course_id, "broken"
))
continue
# if we are in a Microsite, then filter out anything that is not # if we are in a Microsite, then filter out anything that is not
# attributed (by ORG) to that Microsite # attributed (by ORG) to that Microsite
if course_org_filter and course_org_filter != course.location.org: if course_org_filter and course_org_filter != course_overview.location.org:
continue continue
# Conversely, if we are not in a Microsite, then let's filter out any enrollments # Conversely, if we are not in a Microsite, then let's filter out any enrollments
# with courses attributed (by ORG) to Microsites # with courses attributed (by ORG) to Microsites
elif course.location.org in org_filter_out_set: elif course_overview.location.org in org_filter_out_set:
continue continue
# If, somehow, we've got a ccx that has been created for a # If, somehow, we've got a ccx that has been created for a
# course with a deprecated ID, we must filter it out. Emit a # course with a deprecated ID, we must filter it out. Emit a
# warning to the log so we can clean up. # warning to the log so we can clean up.
if course.location.deprecated: if course_overview.location.deprecated:
log.warning( log.warning(
"CCX %s exists for course %s with deprecated id", "CCX %s exists for course %s with deprecated id",
ccx, ccx,
...@@ -270,8 +283,4 @@ def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set): ...@@ -270,8 +283,4 @@ def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set):
) )
continue continue
yield (ccx, membership, course) yield (ccx, membership, course_overview)
else:
log.error("User {0} enrolled in {2} course {1}".format( # pylint: disable=logging-format-interpolation
user.username, ccx.course_id, "broken" if course else "non-existent"
))
...@@ -42,7 +42,7 @@ from ccx_keys.locator import CCXLocator ...@@ -42,7 +42,7 @@ from ccx_keys.locator import CCXLocator
% endif % endif
</h3> </h3>
<div class="course-info"> <div class="course-info">
<span class="info-university">${get_course_about_section(course, 'university')} - </span> <span class="info-university">${get_course_about_section(course_overview, 'university')} - </span>
<span class="info-course-id">${course_overview.display_number_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"> <span class="info-date-block" data-tooltip="Hi">
% if ccx.has_ended(): % if ccx.has_ended():
......
...@@ -90,10 +90,10 @@ from django.core.urlresolvers import reverse ...@@ -90,10 +90,10 @@ from django.core.urlresolvers import reverse
% endfor % endfor
% if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): % if settings.FEATURES.get('CUSTOM_COURSES_EDX', False):
% for ccx, membership, course in ccx_membership_triplets: % for ccx, membership, course_overview in ccx_membership_triplets:
<% show_courseware_link = ccx.has_started() %> <% show_courseware_link = ccx.has_started() %>
<% is_course_blocked = False %> <% is_course_blocked = False %>
<%include file='ccx/_dashboard_ccx_listing.html' args="ccx=ccx, membership=membership, course_overview=enrollment.course_overview, show_courseware_link=show_courseware_link, is_course_blocked=is_course_blocked" /> <%include file='ccx/_dashboard_ccx_listing.html' args="ccx=ccx, membership=membership, course_overview=course_overview, show_courseware_link=show_courseware_link, is_course_blocked=is_course_blocked" />
% endfor % endfor
% endif % endif
......
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