Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

CCX: Use Modulestore Field Overrides instead of User-specific Overrides

Browse Source
This commit is contained in:
Nimisha Asthagiri
2016-06-21 13:52:07 -04:00
parent 74a3665289
commit d0fcabd407
10 changed files with 119 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
lms/djangoapps/ccx/overrides.py
View File

@@ -50,12 +50,11 @@ class CustomCoursesForEdxOverrideProvider(FieldOverrideProvider):
return default
@classmethod
def enabled_for(cls, course):
"""CCX field overrides are enabled per-course
protect against missing attributes
def enabled_for(cls, block):
"""
return getattr(course, 'enable_ccx', False)
CCX field overrides are enabled for CCX blocks.
"""
return getattr(block.location, 'ccx', None) or getattr(block, 'enable_ccx', False)
def get_current_ccx(course_key):
@@ -86,12 +85,9 @@ def get_override_for_ccx(ccx, block, name, default=None):
"""
overrides = _get_overrides_for_ccx(ccx)
if isinstance(block.location, CCXBlockUsageLocator):
non_ccx_key = block.location.to_block_locator()
else:
non_ccx_key = block.location
clean_ccx_key = _clean_ccx_key(block.location)
block_overrides = overrides.get(non_ccx_key, {})
block_overrides = overrides.get(clean_ccx_key, {})
if name in block_overrides:
try:
return block.fields[name].from_json(block_overrides[name])
@@ -101,6 +97,21 @@ def get_override_for_ccx(ccx, block, name, default=None):
return default
def _clean_ccx_key(block_location):
"""
Converts the given BlockUsageKey from a CCX key to the
corresponding key for its parent course, while handling the case
where no conversion is needed. Also strips any version and
branch information from the key.
Returns the cleaned key.
"""
if isinstance(block_location, CCXBlockUsageLocator):
clean_key = block_location.to_block_locator()
else:
clean_key = block_location
return clean_key.version_agnostic().for_branch(None)
def _get_overrides_for_ccx(ccx):
"""
Returns a dictionary mapping field name to overriden value for any
@@ -136,6 +147,7 @@ def override_field_for_ccx(ccx, block, name, value):
value_json = field.to_json(value)
serialized_value = json.dumps(value_json)
override_has_changes = False
clean_ccx_key = _clean_ccx_key(block.location)
override = get_override_for_ccx(ccx, block, name + "_instance")
if override:
@@ -149,7 +161,7 @@ def override_field_for_ccx(ccx, block, name, value):
defaults={'value': serialized_value},
)
if created:
_get_overrides_for_ccx(ccx).setdefault(block.location, {})[name + "_id"] = override.id
_get_overrides_for_ccx(ccx).setdefault(clean_ccx_key, {})[name + "_id"] = override.id
else:
override_has_changes = serialized_value != override.value
@@ -157,8 +169,8 @@ def override_field_for_ccx(ccx, block, name, value):
override.value = serialized_value
override.save()
_get_overrides_for_ccx(ccx).setdefault(block.location, {})[name] = value_json
_get_overrides_for_ccx(ccx).setdefault(block.location, {})[name + "_instance"] = override
_get_overrides_for_ccx(ccx).setdefault(clean_ccx_key, {})[name] = value_json
_get_overrides_for_ccx(ccx).setdefault(clean_ccx_key, {})[name + "_instance"] = override
def clear_override_for_ccx(ccx, block, name):
@@ -185,7 +197,8 @@ def clear_ccx_field_info_from_ccx_map(ccx, block, name): # pylint: disable=inva
Remove field information from ccx overrides mapping dictionary
"""
try:
ccx_override_map = _get_overrides_for_ccx(ccx).setdefault(block.location, {})
clean_ccx_key = _clean_ccx_key(block.location)
ccx_override_map = _get_overrides_for_ccx(ccx).setdefault(clean_ccx_key, {})
ccx_override_map.pop(name)
ccx_override_map.pop(name + "_id")
ccx_override_map.pop(name + "_instance")

90
lms/djangoapps/ccx/tests/test_field_override_performance.py
View File

@@ -9,6 +9,7 @@ from nose.plugins.skip import SkipTest
from courseware.views.views import progress
from courseware.field_overrides import OverrideFieldData
from courseware.testutils import FieldOverrideTestMixin
from datetime import datetime
from django.conf import settings
from django.core.cache import caches
@@ -20,13 +21,13 @@ from request_cache.middleware import RequestCache
from student.models import CourseEnrollment
from student.tests.factories import UserFactory
from xblock.core import XBlock
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \
TEST_DATA_SPLIT_MODULESTORE, TEST_DATA_MONGO_MODULESTORE
from xmodule.modulestore.tests.factories import check_mongo_calls_range, CourseFactory, check_sum_of_calls
from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin
from ccx_keys.locator import CCXLocator
from lms.djangoapps.ccx.tests.factories import CcxFactory
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
@attr('shard_3')
@@ -38,8 +39,7 @@ from lms.djangoapps.ccx.tests.factories import CcxFactory
}
)
@ddt.ddt
class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
ModuleStoreTestCase):
class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseTestMixin, ModuleStoreTestCase):
"""
Base class for instrumenting SQL queries and Mongo reads for field override
providers.
@@ -51,8 +51,6 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
# TEST_DATA must be overridden by subclasses
TEST_DATA = None
ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
def setUp(self):
"""
Create a test client, course, and user.
@@ -172,7 +170,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
caches[cache].clear()
# Refill the metadata inheritance cache
modulestore().get_course(self.course.id, depth=None)
get_course_in_cache(self.course.id)
# We clear the request cache to simulate a new request in the LMS.
RequestCache.clear_request_cache()
@@ -190,7 +188,8 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
@ddt.data(*itertools.product(('no_overrides', 'ccx'), range(1, 4), (True, False), (True, False)))
@ddt.unpack
@override_settings(
FIELD_OVERRIDE_PROVIDERS=(),
XBLOCK_FIELD_DATA_WRAPPERS=[],
MODULESTORE_FIELD_OVERRIDE_PROVIDERS=[],
)
def test_field_overrides(self, overrides, course_width, enable_ccx, view_as_ccx):
"""
@@ -209,7 +208,10 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
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(
XBLOCK_FIELD_DATA_WRAPPERS=['lms.djangoapps.courseware.field_overrides:OverrideModulestoreFieldData.wrap'],
MODULESTORE_FIELD_OVERRIDE_PROVIDERS=providers[overrides],
):
default_queries, history_queries, reads, xblocks = self.TEST_DATA[
(overrides, course_width, enable_ccx, view_as_ccx)
]
@@ -232,24 +234,24 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
# # of mongo queries,
# # of xblocks
# )
('no_overrides', 1, True, False): (47, 1, 6, 13),
('no_overrides', 2, True, False): (119, 16, 6, 84),
('no_overrides', 3, True, False): (399, 81, 6, 335),
('ccx', 1, True, False): (47, 1, 6, 13),
('ccx', 2, True, False): (119, 16, 6, 84),
('ccx', 3, True, False): (399, 81, 6, 335),
('ccx', 1, True, True): (47, 1, 6, 13),
('ccx', 2, True, True): (119, 16, 6, 84),
('ccx', 3, True, True): (399, 81, 6, 335),
('no_overrides', 1, False, False): (47, 1, 6, 13),
('no_overrides', 2, False, False): (119, 16, 6, 84),
('no_overrides', 3, False, False): (399, 81, 6, 335),
('ccx', 1, False, False): (47, 1, 6, 13),
('ccx', 2, False, False): (119, 16, 6, 84),
('ccx', 3, False, False): (399, 81, 6, 335),
('ccx', 1, False, True): (47, 1, 6, 13),
('ccx', 2, False, True): (119, 16, 6, 84),
('ccx', 3, False, True): (399, 81, 6, 335),
('no_overrides', 1, True, False): (34, 0, 6, 1),
('no_overrides', 2, True, False): (40, 0, 6, 1),
('no_overrides', 3, True, False): (50, 0, 6, 1),
('ccx', 1, True, False): (34, 0, 6, 1),
('ccx', 2, True, False): (40, 0, 6, 1),
('ccx', 3, True, False): (50, 0, 6, 1),
('ccx', 1, True, True): (47, 0, 6, 1),
('ccx', 2, True, True): (40, 0, 6, 1),
('ccx', 3, True, True): (50, 0, 6, 1),
('no_overrides', 1, False, False): (34, 0, 6, 1),
('no_overrides', 2, False, False): (40, 0, 6, 1),
('no_overrides', 3, False, False): (50, 0, 6, 1),
('ccx', 1, False, False): (34, 0, 6, 1),
('ccx', 2, False, False): (40, 0, 6, 1),
('ccx', 3, False, False): (50, 0, 6, 1),
('ccx', 1, False, True): (47, 0, 6, 1),
('ccx', 2, False, True): (40, 0, 6, 1),
('ccx', 3, False, True): (50, 0, 6, 1),
}
@@ -261,22 +263,22 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
TEST_DATA = {
('no_overrides', 1, True, False): (47, 1, 4, 9),
('no_overrides', 2, True, False): (119, 16, 19, 54),
('no_overrides', 3, True, False): (399, 81, 84, 215),
('ccx', 1, True, False): (47, 1, 4, 9),
('ccx', 2, True, False): (119, 16, 19, 54),
('ccx', 3, True, False): (399, 81, 84, 215),
('ccx', 1, True, True): (49, 1, 4, 13),
('ccx', 2, True, True): (121, 16, 19, 84),
('ccx', 3, True, True): (401, 81, 84, 335),
('no_overrides', 1, False, False): (47, 1, 4, 9),
('no_overrides', 2, False, False): (119, 16, 19, 54),
('no_overrides', 3, False, False): (399, 81, 84, 215),
('ccx', 1, False, False): (47, 1, 4, 9),
('ccx', 2, False, False): (119, 16, 19, 54),
('ccx', 3, False, False): (399, 81, 84, 215),
('ccx', 1, False, True): (47, 1, 4, 9),
('ccx', 2, False, True): (119, 16, 19, 54),
('ccx', 3, False, True): (399, 81, 84, 215),
('no_overrides', 1, True, False): (34, 0, 4, 1),
('no_overrides', 2, True, False): (40, 0, 19, 1),
('no_overrides', 3, True, False): (50, 0, 84, 1),
('ccx', 1, True, False): (34, 0, 4, 1),
('ccx', 2, True, False): (40, 0, 19, 1),
('ccx', 3, True, False): (50, 0, 84, 1),
('ccx', 1, True, True): (35, 0, 5, 6),
('ccx', 2, True, True): (41, 0, 20, 47),
('ccx', 3, True, True): (51, 0, 85, 202),
('no_overrides', 1, False, False): (34, 0, 4, 1),
('no_overrides', 2, False, False): (40, 0, 19, 1),
('no_overrides', 3, False, False): (50, 0, 84, 1),
('ccx', 1, False, False): (34, 0, 4, 1),
('ccx', 2, False, False): (40, 0, 19, 1),
('ccx', 3, False, False): (50, 0, 84, 1),
('ccx', 1, False, True): (46, 0, 4, 1),
('ccx', 2, False, True): (118, 0, 19, 1),
('ccx', 3, False, True): (398, 0, 84, 1),
}

28
lms/djangoapps/ccx/tests/test_overrides.py
View File

@@ -7,7 +7,10 @@ import mock
import pytz
from nose.plugins.attrib import attr
from ccx_keys.locator import CCXLocator
from courseware.courses import get_course_by_id
from courseware.field_overrides import OverrideFieldData
from courseware.testutils import FieldOverrideTestMixin
from django.test.utils import override_settings
from lms.djangoapps.courseware.tests.test_field_overrides import inject_field_overrides
from request_cache.middleware import RequestCache
@@ -24,9 +27,11 @@ from lms.djangoapps.ccx.tests.utils import flatten, iter_blocks
@attr('shard_1')
@override_settings(FIELD_OVERRIDE_PROVIDERS=(
'ccx.overrides.CustomCoursesForEdxOverrideProvider',))
class TestFieldOverrides(SharedModuleStoreTestCase):
@override_settings(
XBLOCK_FIELD_DATA_WRAPPERS=['lms.djangoapps.courseware.field_overrides:OverrideModulestoreFieldData.wrap'],
MODULESTORE_FIELD_OVERRIDE_PROVIDERS=['ccx.overrides.CustomCoursesForEdxOverrideProvider'],
)
class TestFieldOverrides(FieldOverrideTestMixin, SharedModuleStoreTestCase):
"""
Make sure field overrides behave in the expected manner.
"""
@@ -77,6 +82,9 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
inject_field_overrides(iter_blocks(ccx.course), self.course, AdminFactory.create())
self.ccx_key = CCXLocator.from_course_locator(self.course.id, ccx.id)
self.ccx_course = get_course_by_id(self.ccx_key, depth=None)
def cleanup_provider_classes():
"""
After everything is done, clean up by un-doing the change to the
@@ -90,7 +98,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that overriding start date on a chapter works.
"""
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]
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
self.assertEquals(chapter.start, ccx_start)
@@ -99,7 +107,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that for creating new field executed only create query
"""
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]
# One outer SAVEPOINT/RELEASE SAVEPOINT pair around everything caused by the
# transaction.atomic decorator wrapping override_field_for_ccx.
# One SELECT and one INSERT.
@@ -114,7 +122,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
"""
ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
new_ccx_start = datetime.datetime(2015, 12, 25, 00, 00, tzinfo=pytz.UTC)
chapter = self.ccx.course.get_children()[0]
chapter = self.ccx_course.get_children()[0]
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
with self.assertNumQueries(3):
override_field_for_ccx(self.ccx, chapter, 'start', new_ccx_start)
@@ -124,7 +132,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that if value of field does not changed no query execute.
"""
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]
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
with self.assertNumQueries(2): # 2 savepoints
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
@@ -134,7 +142,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test no extra queries when accessing an overriden field more than once.
"""
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]
# One outer SAVEPOINT/RELEASE SAVEPOINT pair around everything caused by the
# transaction.atomic decorator wrapping override_field_for_ccx.
# One SELECT and one INSERT.
@@ -148,7 +156,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that sequentials inherit overridden start date from chapter.
"""
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]
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
self.assertEquals(chapter.get_children()[0].start, ccx_start)
self.assertEquals(chapter.get_children()[1].start, ccx_start)
@@ -160,7 +168,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
the mooc.
"""
ccx_due = datetime.datetime(2015, 1, 1, 00, 00, tzinfo=pytz.UTC)
chapter = self.ccx.course.get_children()[0]
chapter = self.ccx_course.get_children()[0]
chapter.display_name = 'itsme!'
override_field_for_ccx(self.ccx, chapter, 'due', ccx_due)
vertical = chapter.get_children()[0].get_children()[0]

9
lms/djangoapps/ccx/tests/test_views.py
View File

@@ -15,6 +15,7 @@ from courseware.courses import get_course_by_id
from courseware.tests.factories import StudentModuleFactory
from courseware.tests.helpers import LoginEnrollmentTestCase
from courseware.tabs import get_course_tab_list
from courseware.testutils import FieldOverrideTestMixin
from instructor.access import (
allow_access,
list_with_level,
@@ -921,10 +922,12 @@ def patched_get_children(self, usage_key_filter=None):
@attr('shard_1')
@override_settings(FIELD_OVERRIDE_PROVIDERS=(
'ccx.overrides.CustomCoursesForEdxOverrideProvider',))
@override_settings(
XBLOCK_FIELD_DATA_WRAPPERS=['lms.djangoapps.courseware.field_overrides:OverrideModulestoreFieldData.wrap'],
MODULESTORE_FIELD_OVERRIDE_PROVIDERS=['ccx.overrides.CustomCoursesForEdxOverrideProvider'],
)
@patch('xmodule.x_module.XModuleMixin.get_children', patched_get_children, spec=True)
class TestCCXGrades(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
class TestCCXGrades(FieldOverrideTestMixin, SharedModuleStoreTestCase, LoginEnrollmentTestCase):
"""
Tests for Custom Courses views.
"""

6
lms/djangoapps/ccx/utils.py
View File

@@ -282,12 +282,6 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke
def prep_course_for_grading(course, request):
"""Set up course module for overrides to function properly"""
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
course.id, request.user, course, depth=2)
course = get_module_for_descriptor(
request.user, request, course, field_data_cache, course.id, course=course
)
course._field_data_cache = {} # pylint: disable=protected-access
course.set_grading_policy(course.grading_policy)

11
lms/djangoapps/courseware/tests/test_field_overrides.py
View File

@@ -17,6 +17,7 @@ from ..field_overrides import (
OverrideFieldData,
OverrideModulestoreFieldData,
)
from ..testutils import FieldOverrideTestMixin
TESTUSER = "testuser"
@@ -128,15 +129,7 @@ class OverrideFieldDataTests(SharedModuleStoreTestCase):
@override_settings(
MODULESTORE_FIELD_OVERRIDE_PROVIDERS=['courseware.tests.test_field_overrides.TestOverrideProvider']
)
class OverrideModulestoreFieldDataTests(OverrideFieldDataTests):
def setUp(self):
super(OverrideModulestoreFieldDataTests, self).setUp()
OverrideModulestoreFieldData.provider_classes = None
def tearDown(self):
super(OverrideModulestoreFieldDataTests, self).tearDown()
OverrideModulestoreFieldData.provider_classes = None
class OverrideModulestoreFieldDataTests(FieldOverrideTestMixin, OverrideFieldDataTests):
def make_one(self):
return OverrideModulestoreFieldData.wrap(self.course, DictFieldData({
'foo': 'bar',

8
lms/djangoapps/courseware/tests/test_module_render.py
View File

@@ -339,16 +339,16 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
)
@override_settings(FIELD_OVERRIDE_PROVIDERS=(
'ccx.overrides.CustomCoursesForEdxOverrideProvider',
'courseware.student_field_overrides.IndividualStudentOverrideProvider',
))
def test_rebind_different_users_ccx(self):
def test_rebind_different_users(self):
"""
This tests the rebinding a descriptor to a student does not result
in overly nested _field_data when CCX is enabled.
in overly nested _field_data.
"""
request = self.request_factory.get('')
request.user = self.mock_user
course = CourseFactory.create(enable_ccx=True)
course = CourseFactory.create()
descriptor = ItemFactory(category='html', parent=course)
field_data_cache = FieldDataCache(

14
lms/djangoapps/courseware/testutils.py
View File

@@ -8,6 +8,7 @@ import ddt
from mock import patch
from urllib import urlencode
from lms.djangoapps.courseware.field_overrides import OverrideModulestoreFieldData
from lms.djangoapps.courseware.url_helpers import get_redirect_url
from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory
from xmodule.modulestore import ModuleStoreEnum
@@ -197,3 +198,16 @@ class RenderXBlockTestMixin(object):
self.setup_course()
self.setup_user(admin=False, enroll=True, login=True)
self.verify_response(url_params={'view': 'author_view'}, expected_response_code=400)
class FieldOverrideTestMixin(object):
"""
A Mixin helper class for classes that test Field Overrides.
"""
def setUp(self):
super(FieldOverrideTestMixin, self).setUp()
OverrideModulestoreFieldData.provider_classes = None
def tearDown(self):
super(FieldOverrideTestMixin, self).tearDown()
OverrideModulestoreFieldData.provider_classes = None

2
lms/envs/aws.py
View File

@@ -731,7 +731,7 @@ COURSE_CATALOG_API_URL = ENV_TOKENS.get('COURSE_CATALOG_API_URL', COURSE_CATALOG
##### Custom Courses for EdX #####
if FEATURES.get('CUSTOM_COURSES_EDX'):
INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon')
FIELD_OVERRIDE_PROVIDERS += (
MODULESTORE_FIELD_OVERRIDE_PROVIDERS += (
'lms.djangoapps.ccx.overrides.CustomCoursesForEdxOverrideProvider',
)
CCX_MAX_STUDENTS_ALLOWED = ENV_TOKENS.get('CCX_MAX_STUDENTS_ALLOWED', CCX_MAX_STUDENTS_ALLOWED)

2
lms/envs/yaml_config.py
View File

@@ -299,7 +299,7 @@ GRADES_DOWNLOAD_ROUTING_KEY = HIGH_MEM_QUEUE
##### Custom Courses for EdX #####
if FEATURES.get('CUSTOM_COURSES_EDX'):
INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon')
FIELD_OVERRIDE_PROVIDERS += (
MODULESTORE_FIELD_OVERRIDE_PROVIDERS += (
'lms.djangoapps.ccx.overrides.CustomCoursesForEdxOverrideProvider',
)