Commit 1798b1f1 by Nimisha Asthagiri Committed by GitHub

Merge pull request #12567 from edx/tnl/grading

Grading uses Block Transformers
parents b42a9ff4 7fe002ff
...@@ -101,7 +101,7 @@ class TestOrphan(TestOrphanBase): ...@@ -101,7 +101,7 @@ class TestOrphan(TestOrphanBase):
@ddt.data( @ddt.data(
(ModuleStoreEnum.Type.split, 9, 6), (ModuleStoreEnum.Type.split, 9, 6),
(ModuleStoreEnum.Type.mongo, 30, 13), (ModuleStoreEnum.Type.mongo, 34, 13),
) )
@ddt.unpack @ddt.unpack
def test_delete_orphans(self, default_store, max_mongo_calls, min_mongo_calls): def test_delete_orphans(self, default_store, max_mongo_calls, min_mongo_calls):
......
"""
Simple utility functions that operate on block metadata.
This is a place to put simple functions that operate on block metadata. It
allows us to share code between the XModuleMixin and CourseOverview and
BlockStructure.
"""
def url_name_for_block(block):
"""
Given a block, returns the block's URL name.
Arguments:
block (XModuleMixin|CourseOverview|BlockStructureBlockData):
Block that is being accessed
"""
return block.location.name
def display_name_with_default(block):
"""
Calculates the display name for a block.
Default to the display_name if it isn't None, else fall back to creating
a name based on the URL.
Unlike the rest of this module's functions, this function takes an entire
course descriptor/overview as a parameter. This is because a few test cases
(specifically, {Text|Image|Video}AnnotationModuleTestCase.test_student_view)
create scenarios where course.display_name is not None but course.location
is None, which causes calling course.url_name to fail. So, although we'd
like to just pass course.display_name and course.url_name as arguments to
this function, we can't do so without breaking those tests.
Note: This method no longer escapes as it once did, so the caller must
ensure it is properly escaped where necessary.
Arguments:
block (XModuleMixin|CourseOverview|BlockStructureBlockData):
Block that is being accessed
"""
return (
block.display_name if block.display_name is not None
else url_name_for_block(block).replace('_', ' ')
)
def display_name_with_default_escaped(block):
"""
DEPRECATED: use display_name_with_default
Calculates the display name for a block with some HTML escaping.
This follows the same logic as display_name_with_default, with
the addition of the escaping.
Here is an example of how to move away from this method in Mako html:
Before:
<span class="course-name">${course.display_name_with_default_escaped}</span>
After:
<span class="course-name">${course.display_name_with_default | h}</span>
If the context is Javascript in Mako, you'll need to follow other best practices.
Note: Switch to display_name_with_default, and ensure the caller
properly escapes where necessary.
Note: This newly introduced method should not be used. It was only
introduced to enable a quick search/replace and the ability to slowly
migrate and test switching to display_name_with_default, which is no
longer escaped.
Arguments:
block (XModuleMixin|CourseOverview|BlockStructureBlockData):
Block that is being accessed
"""
# This escaping is incomplete. However, rather than switching this to use
# markupsafe.escape() and fixing issues, better to put that energy toward
# migrating away from this method altogether.
return display_name_with_default(block).replace('<', '&lt;').replace('>', '&gt;')
...@@ -32,78 +32,6 @@ def clean_course_key(course_key, padding_char): ...@@ -32,78 +32,6 @@ def clean_course_key(course_key, padding_char):
) )
def url_name_for_course_location(location):
"""
Given a course's usage locator, returns the course's URL name.
Arguments:
location (BlockUsageLocator): The course's usage locator.
"""
return location.name
def display_name_with_default(course):
"""
Calculates the display name for a course.
Default to the display_name if it isn't None, else fall back to creating
a name based on the URL.
Unlike the rest of this module's functions, this function takes an entire
course descriptor/overview as a parameter. This is because a few test cases
(specifically, {Text|Image|Video}AnnotationModuleTestCase.test_student_view)
create scenarios where course.display_name is not None but course.location
is None, which causes calling course.url_name to fail. So, although we'd
like to just pass course.display_name and course.url_name as arguments to
this function, we can't do so without breaking those tests.
Note: This method no longer escapes as it once did, so the caller must
ensure it is properly escaped where necessary.
Arguments:
course (CourseDescriptor|CourseOverview): descriptor or overview of
said course.
"""
return (
course.display_name if course.display_name is not None
else course.url_name.replace('_', ' ')
)
def display_name_with_default_escaped(course):
"""
DEPRECATED: use display_name_with_default
Calculates the display name for a course with some HTML escaping.
This follows the same logic as display_name_with_default, with
the addition of the escaping.
Here is an example of how to move away from this method in Mako html:
Before:
<span class="course-name">${course.display_name_with_default_escaped}</span>
After:
<span class="course-name">${course.display_name_with_default | h}</span>
If the context is Javascript in Mako, you'll need to follow other best practices.
Note: Switch to display_name_with_default, and ensure the caller
properly escapes where necessary.
Note: This newly introduced method should not be used. It was only
introduced to enable a quick search/replace and the ability to slowly
migrate and test switching to display_name_with_default, which is no
longer escaped.
Arguments:
course (CourseDescriptor|CourseOverview): descriptor or overview of
said course.
"""
# This escaping is incomplete. However, rather than switching this to use
# markupsafe.escape() and fixing issues, better to put that energy toward
# migrating away from this method altogether.
return course.display_name_with_default.replace('<', '&lt;').replace('>', '&gt;')
def number_for_course_location(location): def number_for_course_location(location):
""" """
Given a course's block usage locator, returns the course's number. Given a course's block usage locator, returns the course's number.
......
...@@ -11,12 +11,10 @@ from django.utils.timezone import UTC ...@@ -11,12 +11,10 @@ from django.utils.timezone import UTC
from lazy import lazy from lazy import lazy
from lxml import etree from lxml import etree
from path import Path as path from path import Path as path
from xblock.core import XBlock
from xblock.fields import Scope, List, String, Dict, Boolean, Integer, Float from xblock.fields import Scope, List, String, Dict, Boolean, Integer, Float
from xmodule import course_metadata_utils from xmodule import course_metadata_utils
from xmodule.course_metadata_utils import DEFAULT_START_DATE from xmodule.course_metadata_utils import DEFAULT_START_DATE
from xmodule.exceptions import UndefinedContext
from xmodule.graders import grader_from_conf from xmodule.graders import grader_from_conf
from xmodule.mixin import LicenseMixin from xmodule.mixin import LicenseMixin
from xmodule.seq_module import SequenceDescriptor, SequenceModule from xmodule.seq_module import SequenceDescriptor, SequenceModule
...@@ -1183,83 +1181,6 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin): ...@@ -1183,83 +1181,6 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin):
""" """
return course_metadata_utils.sorting_score(self.start, self.advertised_start, self.announcement) return course_metadata_utils.sorting_score(self.start, self.advertised_start, self.announcement)
@lazy
def grading_context(self):
"""
This returns a dictionary with keys necessary for quickly grading
a student. They are used by grades.grade()
The grading context has two keys:
graded_sections - This contains the sections that are graded, as
well as all possible children modules that can affect the
grading. This allows some sections to be skipped if the student
hasn't seen any part of it.
The format is a dictionary keyed by section-type. The values are
arrays of dictionaries containing
"section_descriptor" : The section descriptor
"xmoduledescriptors" : An array of xmoduledescriptors that
could possibly be in the section, for any student
all_descriptors - This contains a list of all xmodules that can
effect grading a student. This is used to efficiently fetch
all the xmodule state for a FieldDataCache without walking
the descriptor tree again.
"""
# If this descriptor has been bound to a student, return the corresponding
# XModule. If not, just use the descriptor itself
try:
module = getattr(self, '_xmodule', None)
if not module:
module = self
except UndefinedContext:
module = self
def possibly_scored(usage_key):
"""Can this XBlock type can have a score or children?"""
return usage_key.block_type in self.block_types_affecting_grading
all_descriptors = []
graded_sections = {}
def yield_descriptor_descendents(module_descriptor):
for child in module_descriptor.get_children(usage_key_filter=possibly_scored):
yield child
for module_descriptor in yield_descriptor_descendents(child):
yield module_descriptor
for chapter in self.get_children():
for section in chapter.get_children():
if section.graded:
xmoduledescriptors = list(yield_descriptor_descendents(section))
xmoduledescriptors.append(section)
# The xmoduledescriptors included here are only the ones that have scores.
section_description = {
'section_descriptor': section,
'xmoduledescriptors': [child for child in xmoduledescriptors if child.has_score]
}
section_format = section.format if section.format is not None else ''
graded_sections[section_format] = graded_sections.get(section_format, []) + [section_description]
all_descriptors.extend(xmoduledescriptors)
all_descriptors.append(section)
return {'graded_sections': graded_sections,
'all_descriptors': all_descriptors, }
@lazy
def block_types_affecting_grading(self):
"""Return all block types that could impact grading (i.e. scored, or having children)."""
return frozenset(
cat for (cat, xblock_class) in XBlock.load_classes() if (
getattr(xblock_class, 'has_score', False) or getattr(xblock_class, 'has_children', False)
)
)
@staticmethod @staticmethod
def make_id(org, course, url_name): def make_id(org, course, url_name):
return '/'.join([org, course, url_name]) return '/'.join([org, course, url_name])
......
...@@ -173,7 +173,7 @@ class WeightedSubsectionsGrader(CourseGrader): ...@@ -173,7 +173,7 @@ class WeightedSubsectionsGrader(CourseGrader):
All items in section_breakdown for each subgrader will be combined. A grade_breakdown will be All items in section_breakdown for each subgrader will be combined. A grade_breakdown will be
composed using the score from each grader. composed using the score from each grader.
Note that the sum of the weights is not take into consideration. If the weights add up to Note that the sum of the weights is not taken into consideration. If the weights add up to
a value > 1, the student may end up with a percent > 100%. This allows for sections that a value > 1, the student may end up with a percent > 100%. This allows for sections that
are extra credit. are extra credit.
""" """
......
...@@ -2950,10 +2950,11 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ...@@ -2950,10 +2950,11 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
output_fields = dict(jsonfields) output_fields = dict(jsonfields)
for field_name, value in output_fields.iteritems(): for field_name, value in output_fields.iteritems():
if value: if value:
field = xblock_class.fields.get(field_name) try:
if field is None: field = xblock_class.fields.get(field_name)
except AttributeError:
continue continue
elif isinstance(field, Reference): if isinstance(field, Reference):
output_fields[field_name] = robust_usage_key(value) output_fields[field_name] = robust_usage_key(value)
elif isinstance(field, ReferenceList): elif isinstance(field, ReferenceList):
output_fields[field_name] = [robust_usage_key(ele) for ele in value] output_fields[field_name] = [robust_usage_key(ele) for ele in value]
......
...@@ -218,7 +218,7 @@ class ModuleStoreIsolationMixin(CacheIsolationMixin): ...@@ -218,7 +218,7 @@ class ModuleStoreIsolationMixin(CacheIsolationMixin):
MODULESTORE = functools.partial(mixed_store_config, mkdtemp_clean(), {}) MODULESTORE = functools.partial(mixed_store_config, mkdtemp_clean(), {})
CONTENTSTORE = functools.partial(contentstore_config) CONTENTSTORE = functools.partial(contentstore_config)
ENABLED_CACHES = ['mongo_metadata_inheritance', 'loc_cache'] ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
__settings_overrides = [] __settings_overrides = []
__old_modulestores = [] __old_modulestores = []
__old_contentstores = [] __old_contentstores = []
......
...@@ -7,11 +7,13 @@ from unittest import TestCase ...@@ -7,11 +7,13 @@ from unittest import TestCase
from django.utils.timezone import UTC from django.utils.timezone import UTC
from xmodule.course_metadata_utils import ( from xmodule.block_metadata_utils import (
clean_course_key, url_name_for_block,
url_name_for_course_location,
display_name_with_default, display_name_with_default,
display_name_with_default_escaped, display_name_with_default_escaped,
)
from xmodule.course_metadata_utils import (
clean_course_key,
number_for_course_location, number_for_course_location,
has_course_started, has_course_started,
has_course_ended, has_course_ended,
...@@ -130,9 +132,9 @@ class CourseMetadataUtilsTestCase(TestCase): ...@@ -130,9 +132,9 @@ class CourseMetadataUtilsTestCase(TestCase):
"course_MNXXK4TTMUWXMMJ2KVXGS5TFOJZWS5DZLAVUGUZNGIYDGK2ZGIYDSNQ~" "course_MNXXK4TTMUWXMMJ2KVXGS5TFOJZWS5DZLAVUGUZNGIYDGK2ZGIYDSNQ~"
), ),
]), ]),
FunctionTest(url_name_for_course_location, [ FunctionTest(url_name_for_block, [
TestScenario((self.demo_course.location,), self.demo_course.location.name), TestScenario((self.demo_course,), self.demo_course.location.name),
TestScenario((self.html_course.location,), self.html_course.location.name), TestScenario((self.html_course,), self.html_course.location.name),
]), ]),
FunctionTest(display_name_with_default_escaped, [ FunctionTest(display_name_with_default_escaped, [
# Test course with no display name. # Test course with no display name.
......
...@@ -27,7 +27,7 @@ from xblock.fields import ( ...@@ -27,7 +27,7 @@ from xblock.fields import (
from xblock.fragment import Fragment from xblock.fragment import Fragment
from xblock.runtime import Runtime, IdReader, IdGenerator from xblock.runtime import Runtime, IdReader, IdGenerator
from xmodule import course_metadata_utils from xmodule import block_metadata_utils
from xmodule.fields import RelativeTime from xmodule.fields import RelativeTime
from xmodule.errortracker import exc_info_to_str from xmodule.errortracker import exc_info_to_str
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
...@@ -340,7 +340,7 @@ class XModuleMixin(XModuleFields, XBlock): ...@@ -340,7 +340,7 @@ class XModuleMixin(XModuleFields, XBlock):
@property @property
def url_name(self): def url_name(self):
return course_metadata_utils.url_name_for_course_location(self.location) return block_metadata_utils.url_name_for_block(self)
@property @property
def display_name_with_default(self): def display_name_with_default(self):
...@@ -348,7 +348,7 @@ class XModuleMixin(XModuleFields, XBlock): ...@@ -348,7 +348,7 @@ class XModuleMixin(XModuleFields, XBlock):
Return a display name for the module: use display_name if defined in Return a display name for the module: use display_name if defined in
metadata, otherwise convert the url name. metadata, otherwise convert the url name.
""" """
return course_metadata_utils.display_name_with_default(self) return block_metadata_utils.display_name_with_default(self)
@property @property
def display_name_with_default_escaped(self): def display_name_with_default_escaped(self):
...@@ -363,7 +363,7 @@ class XModuleMixin(XModuleFields, XBlock): ...@@ -363,7 +363,7 @@ class XModuleMixin(XModuleFields, XBlock):
migrate and test switching to display_name_with_default, which is no migrate and test switching to display_name_with_default, which is no
longer escaped. longer escaped.
""" """
return course_metadata_utils.display_name_with_default_escaped(self) return block_metadata_utils.display_name_with_default_escaped(self)
@property @property
def tooltip_title(self): def tooltip_title(self):
......
...@@ -50,12 +50,11 @@ class CustomCoursesForEdxOverrideProvider(FieldOverrideProvider): ...@@ -50,12 +50,11 @@ class CustomCoursesForEdxOverrideProvider(FieldOverrideProvider):
return default return default
@classmethod @classmethod
def enabled_for(cls, course): def enabled_for(cls, block):
"""CCX field overrides are enabled per-course """
CCX field overrides are enabled for CCX blocks.
protect against missing attributes
""" """
return getattr(course, 'enable_ccx', False) return getattr(block.location, 'ccx', None) or getattr(block, 'enable_ccx', False)
def get_current_ccx(course_key): def get_current_ccx(course_key):
...@@ -86,12 +85,9 @@ def get_override_for_ccx(ccx, block, name, default=None): ...@@ -86,12 +85,9 @@ def get_override_for_ccx(ccx, block, name, default=None):
""" """
overrides = _get_overrides_for_ccx(ccx) overrides = _get_overrides_for_ccx(ccx)
if isinstance(block.location, CCXBlockUsageLocator): clean_ccx_key = _clean_ccx_key(block.location)
non_ccx_key = block.location.to_block_locator()
else:
non_ccx_key = block.location
block_overrides = overrides.get(non_ccx_key, {}) block_overrides = overrides.get(clean_ccx_key, {})
if name in block_overrides: if name in block_overrides:
try: try:
return block.fields[name].from_json(block_overrides[name]) return block.fields[name].from_json(block_overrides[name])
...@@ -101,6 +97,21 @@ def get_override_for_ccx(ccx, block, name, default=None): ...@@ -101,6 +97,21 @@ def get_override_for_ccx(ccx, block, name, default=None):
return default 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): 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
...@@ -136,6 +147,7 @@ def override_field_for_ccx(ccx, block, name, value): ...@@ -136,6 +147,7 @@ def override_field_for_ccx(ccx, block, name, value):
value_json = field.to_json(value) value_json = field.to_json(value)
serialized_value = json.dumps(value_json) serialized_value = json.dumps(value_json)
override_has_changes = False override_has_changes = False
clean_ccx_key = _clean_ccx_key(block.location)
override = get_override_for_ccx(ccx, block, name + "_instance") override = get_override_for_ccx(ccx, block, name + "_instance")
if override: if override:
...@@ -149,7 +161,7 @@ def override_field_for_ccx(ccx, block, name, value): ...@@ -149,7 +161,7 @@ def override_field_for_ccx(ccx, block, name, value):
defaults={'value': serialized_value}, defaults={'value': serialized_value},
) )
if created: 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: else:
override_has_changes = serialized_value != override.value override_has_changes = serialized_value != override.value
...@@ -157,8 +169,8 @@ def override_field_for_ccx(ccx, block, name, value): ...@@ -157,8 +169,8 @@ def override_field_for_ccx(ccx, block, name, value):
override.value = serialized_value override.value = serialized_value
override.save() override.save()
_get_overrides_for_ccx(ccx).setdefault(block.location, {})[name] = value_json _get_overrides_for_ccx(ccx).setdefault(clean_ccx_key, {})[name] = value_json
_get_overrides_for_ccx(ccx).setdefault(block.location, {})[name + "_instance"] = override _get_overrides_for_ccx(ccx).setdefault(clean_ccx_key, {})[name + "_instance"] = override
def clear_override_for_ccx(ccx, block, name): 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 ...@@ -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 Remove field information from ccx overrides mapping dictionary
""" """
try: 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)
ccx_override_map.pop(name + "_id") ccx_override_map.pop(name + "_id")
ccx_override_map.pop(name + "_instance") ccx_override_map.pop(name + "_instance")
......
...@@ -9,6 +9,7 @@ from nose.plugins.skip import SkipTest ...@@ -9,6 +9,7 @@ from nose.plugins.skip import SkipTest
from courseware.views.views import progress from courseware.views.views import progress
from courseware.field_overrides import OverrideFieldData from courseware.field_overrides import OverrideFieldData
from courseware.testutils import FieldOverrideTestMixin
from datetime import datetime from datetime import datetime
from django.conf import settings from django.conf import settings
from django.core.cache import caches from django.core.cache import caches
...@@ -20,13 +21,13 @@ from request_cache.middleware import RequestCache ...@@ -20,13 +21,13 @@ from request_cache.middleware import RequestCache
from student.models import CourseEnrollment from student.models import CourseEnrollment
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from xblock.core import XBlock from xblock.core import XBlock
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \ 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_range, CourseFactory, check_sum_of_calls from xmodule.modulestore.tests.factories import check_mongo_calls_range, 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_keys.locator import CCXLocator
from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.ccx.tests.factories import CcxFactory
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
@attr('shard_3') @attr('shard_3')
...@@ -38,8 +39,7 @@ from lms.djangoapps.ccx.tests.factories import CcxFactory ...@@ -38,8 +39,7 @@ from lms.djangoapps.ccx.tests.factories import CcxFactory
} }
) )
@ddt.ddt @ddt.ddt
class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseTestMixin, ModuleStoreTestCase):
ModuleStoreTestCase):
""" """
Base class for instrumenting SQL queries and Mongo reads for field override Base class for instrumenting SQL queries and Mongo reads for field override
providers. providers.
...@@ -51,8 +51,6 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -51,8 +51,6 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
# TEST_DATA must be overridden by subclasses # TEST_DATA must be overridden by subclasses
TEST_DATA = None TEST_DATA = None
ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
def setUp(self): def setUp(self):
""" """
Create a test client, course, and user. Create a test client, course, and user.
...@@ -172,7 +170,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -172,7 +170,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
caches[cache].clear() caches[cache].clear()
# Refill the metadata inheritance cache # 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. # We clear the request cache to simulate a new request in the LMS.
RequestCache.clear_request_cache() RequestCache.clear_request_cache()
...@@ -190,7 +188,8 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -190,7 +188,8 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
@ddt.data(*itertools.product(('no_overrides', 'ccx'), range(1, 4), (True, False), (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=(), XBLOCK_FIELD_DATA_WRAPPERS=[],
MODULESTORE_FIELD_OVERRIDE_PROVIDERS=[],
) )
def test_field_overrides(self, overrides, course_width, enable_ccx, view_as_ccx): def test_field_overrides(self, overrides, course_width, enable_ccx, view_as_ccx):
""" """
...@@ -209,7 +208,10 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ...@@ -209,7 +208,10 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
if self.MODULESTORE == TEST_DATA_MONGO_MODULESTORE and view_as_ccx: if self.MODULESTORE == TEST_DATA_MONGO_MODULESTORE and view_as_ccx:
raise SkipTest("Can't use a MongoModulestore test as a CCX course") 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[ default_queries, history_queries, reads, xblocks = self.TEST_DATA[
(overrides, course_width, enable_ccx, view_as_ccx) (overrides, course_width, enable_ccx, view_as_ccx)
] ]
...@@ -232,24 +234,24 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): ...@@ -232,24 +234,24 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
# # of mongo queries, # # of mongo queries,
# # of xblocks # # of xblocks
# ) # )
('no_overrides', 1, True, False): (47, 1, 6, 13), ('no_overrides', 1, True, False): (34, 0, 6, 1),
('no_overrides', 2, True, False): (119, 16, 6, 84), ('no_overrides', 2, True, False): (40, 0, 6, 1),
('no_overrides', 3, True, False): (399, 81, 6, 335), ('no_overrides', 3, True, False): (50, 0, 6, 1),
('ccx', 1, True, False): (47, 1, 6, 13), ('ccx', 1, True, False): (34, 0, 6, 1),
('ccx', 2, True, False): (119, 16, 6, 84), ('ccx', 2, True, False): (40, 0, 6, 1),
('ccx', 3, True, False): (399, 81, 6, 335), ('ccx', 3, True, False): (50, 0, 6, 1),
('ccx', 1, True, True): (47, 1, 6, 13), ('ccx', 1, True, True): (47, 0, 6, 1),
('ccx', 2, True, True): (119, 16, 6, 84), ('ccx', 2, True, True): (40, 0, 6, 1),
('ccx', 3, True, True): (399, 81, 6, 335), ('ccx', 3, True, True): (50, 0, 6, 1),
('no_overrides', 1, False, False): (47, 1, 6, 13), ('no_overrides', 1, False, False): (34, 0, 6, 1),
('no_overrides', 2, False, False): (119, 16, 6, 84), ('no_overrides', 2, False, False): (40, 0, 6, 1),
('no_overrides', 3, False, False): (399, 81, 6, 335), ('no_overrides', 3, False, False): (50, 0, 6, 1),
('ccx', 1, False, False): (47, 1, 6, 13), ('ccx', 1, False, False): (34, 0, 6, 1),
('ccx', 2, False, False): (119, 16, 6, 84), ('ccx', 2, False, False): (40, 0, 6, 1),
('ccx', 3, False, False): (399, 81, 6, 335), ('ccx', 3, False, False): (50, 0, 6, 1),
('ccx', 1, False, True): (47, 1, 6, 13), ('ccx', 1, False, True): (47, 0, 6, 1),
('ccx', 2, False, True): (119, 16, 6, 84), ('ccx', 2, False, True): (40, 0, 6, 1),
('ccx', 3, False, True): (399, 81, 6, 335), ('ccx', 3, False, True): (50, 0, 6, 1),
} }
...@@ -261,22 +263,22 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): ...@@ -261,22 +263,22 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True __test__ = True
TEST_DATA = { TEST_DATA = {
('no_overrides', 1, True, False): (47, 1, 4, 9), ('no_overrides', 1, True, False): (34, 0, 4, 1),
('no_overrides', 2, True, False): (119, 16, 19, 54), ('no_overrides', 2, True, False): (40, 0, 19, 1),
('no_overrides', 3, True, False): (399, 81, 84, 215), ('no_overrides', 3, True, False): (50, 0, 84, 1),
('ccx', 1, True, False): (47, 1, 4, 9), ('ccx', 1, True, False): (34, 0, 4, 1),
('ccx', 2, True, False): (119, 16, 19, 54), ('ccx', 2, True, False): (40, 0, 19, 1),
('ccx', 3, True, False): (399, 81, 84, 215), ('ccx', 3, True, False): (50, 0, 84, 1),
('ccx', 1, True, True): (49, 1, 4, 13), ('ccx', 1, True, True): (35, 0, 5, 6),
('ccx', 2, True, True): (121, 16, 19, 84), ('ccx', 2, True, True): (41, 0, 20, 47),
('ccx', 3, True, True): (401, 81, 84, 335), ('ccx', 3, True, True): (51, 0, 85, 202),
('no_overrides', 1, False, False): (47, 1, 4, 9), ('no_overrides', 1, False, False): (34, 0, 4, 1),
('no_overrides', 2, False, False): (119, 16, 19, 54), ('no_overrides', 2, False, False): (40, 0, 19, 1),
('no_overrides', 3, False, False): (399, 81, 84, 215), ('no_overrides', 3, False, False): (50, 0, 84, 1),
('ccx', 1, False, False): (47, 1, 4, 9), ('ccx', 1, False, False): (34, 0, 4, 1),
('ccx', 2, False, False): (119, 16, 19, 54), ('ccx', 2, False, False): (40, 0, 19, 1),
('ccx', 3, False, False): (399, 81, 84, 215), ('ccx', 3, False, False): (50, 0, 84, 1),
('ccx', 1, False, True): (47, 1, 4, 9), ('ccx', 1, False, True): (46, 0, 4, 1),
('ccx', 2, False, True): (119, 16, 19, 54), ('ccx', 2, False, True): (118, 0, 19, 1),
('ccx', 3, False, True): (399, 81, 84, 215), ('ccx', 3, False, True): (398, 0, 84, 1),
} }
...@@ -7,7 +7,10 @@ import mock ...@@ -7,7 +7,10 @@ import mock
import pytz import pytz
from nose.plugins.attrib import attr 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.field_overrides import OverrideFieldData
from courseware.testutils import FieldOverrideTestMixin
from django.test.utils import override_settings from django.test.utils import override_settings
from lms.djangoapps.courseware.tests.test_field_overrides import inject_field_overrides from lms.djangoapps.courseware.tests.test_field_overrides import inject_field_overrides
from request_cache.middleware import RequestCache from request_cache.middleware import RequestCache
...@@ -24,9 +27,11 @@ from lms.djangoapps.ccx.tests.utils import flatten, iter_blocks ...@@ -24,9 +27,11 @@ from lms.djangoapps.ccx.tests.utils import flatten, iter_blocks
@attr('shard_1') @attr('shard_1')
@override_settings(FIELD_OVERRIDE_PROVIDERS=( @override_settings(
'ccx.overrides.CustomCoursesForEdxOverrideProvider',)) XBLOCK_FIELD_DATA_WRAPPERS=['lms.djangoapps.courseware.field_overrides:OverrideModulestoreFieldData.wrap'],
class TestFieldOverrides(SharedModuleStoreTestCase): MODULESTORE_FIELD_OVERRIDE_PROVIDERS=['ccx.overrides.CustomCoursesForEdxOverrideProvider'],
)
class TestFieldOverrides(FieldOverrideTestMixin, SharedModuleStoreTestCase):
""" """
Make sure field overrides behave in the expected manner. Make sure field overrides behave in the expected manner.
""" """
...@@ -77,6 +82,9 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -77,6 +82,9 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
inject_field_overrides(iter_blocks(ccx.course), self.course, AdminFactory.create()) 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(): def cleanup_provider_classes():
""" """
After everything is done, clean up by un-doing the change to the After everything is done, clean up by un-doing the change to the
...@@ -90,7 +98,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -90,7 +98,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that overriding start date on a chapter works. Test that overriding start date on a chapter works.
""" """
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]
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start) override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
self.assertEquals(chapter.start, ccx_start) self.assertEquals(chapter.start, ccx_start)
...@@ -99,7 +107,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -99,7 +107,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that for creating new field executed only create query Test that for creating new field executed only create query
""" """
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]
# One outer SAVEPOINT/RELEASE SAVEPOINT pair around everything caused by the # One outer SAVEPOINT/RELEASE SAVEPOINT pair around everything caused by the
# transaction.atomic decorator wrapping override_field_for_ccx. # transaction.atomic decorator wrapping override_field_for_ccx.
# One SELECT and one INSERT. # One SELECT and one INSERT.
...@@ -114,7 +122,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -114,7 +122,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
""" """
ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC) 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) 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) override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
with self.assertNumQueries(3): with self.assertNumQueries(3):
override_field_for_ccx(self.ccx, chapter, 'start', new_ccx_start) override_field_for_ccx(self.ccx, chapter, 'start', new_ccx_start)
...@@ -124,7 +132,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -124,7 +132,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that if value of field does not changed no query execute. Test that if value of field does not changed no query execute.
""" """
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]
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start) override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
with self.assertNumQueries(2): # 2 savepoints with self.assertNumQueries(2): # 2 savepoints
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start) override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
...@@ -134,7 +142,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -134,7 +142,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test no extra queries when accessing an overriden field more than once. Test no extra queries when accessing an overriden field more than once.
""" """
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]
# One outer SAVEPOINT/RELEASE SAVEPOINT pair around everything caused by the # One outer SAVEPOINT/RELEASE SAVEPOINT pair around everything caused by the
# transaction.atomic decorator wrapping override_field_for_ccx. # transaction.atomic decorator wrapping override_field_for_ccx.
# One SELECT and one INSERT. # One SELECT and one INSERT.
...@@ -148,7 +156,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -148,7 +156,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
Test that sequentials inherit overridden start date from chapter. Test that sequentials inherit overridden start date from chapter.
""" """
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]
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start) override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
self.assertEquals(chapter.get_children()[0].start, ccx_start) self.assertEquals(chapter.get_children()[0].start, ccx_start)
self.assertEquals(chapter.get_children()[1].start, ccx_start) self.assertEquals(chapter.get_children()[1].start, ccx_start)
...@@ -160,7 +168,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase): ...@@ -160,7 +168,7 @@ class TestFieldOverrides(SharedModuleStoreTestCase):
the mooc. the mooc.
""" """
ccx_due = datetime.datetime(2015, 1, 1, 00, 00, tzinfo=pytz.UTC) 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!' chapter.display_name = 'itsme!'
override_field_for_ccx(self.ccx, chapter, 'due', ccx_due) override_field_for_ccx(self.ccx, chapter, 'due', ccx_due)
vertical = chapter.get_children()[0].get_children()[0] vertical = chapter.get_children()[0].get_children()[0]
......
...@@ -15,6 +15,7 @@ from courseware.courses import get_course_by_id ...@@ -15,6 +15,7 @@ from courseware.courses import get_course_by_id
from courseware.tests.factories import StudentModuleFactory from courseware.tests.factories import StudentModuleFactory
from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.helpers import LoginEnrollmentTestCase
from courseware.tabs import get_course_tab_list from courseware.tabs import get_course_tab_list
from courseware.testutils import FieldOverrideTestMixin
from instructor.access import ( from instructor.access import (
allow_access, allow_access,
list_with_level, list_with_level,
...@@ -921,10 +922,12 @@ def patched_get_children(self, usage_key_filter=None): ...@@ -921,10 +922,12 @@ def patched_get_children(self, usage_key_filter=None):
@attr('shard_1') @attr('shard_1')
@override_settings(FIELD_OVERRIDE_PROVIDERS=( @override_settings(
'ccx.overrides.CustomCoursesForEdxOverrideProvider',)) 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) @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. Tests for Custom Courses views.
""" """
......
...@@ -282,12 +282,6 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke ...@@ -282,12 +282,6 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke
def prep_course_for_grading(course, request): def prep_course_for_grading(course, request):
"""Set up course module for overrides to function properly""" """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._field_data_cache = {} # pylint: disable=protected-access
course.set_grading_policy(course.grading_policy) course.set_grading_policy(course.grading_policy)
......
...@@ -51,7 +51,7 @@ class Command(BaseCommand): ...@@ -51,7 +51,7 @@ class Command(BaseCommand):
for cert in ungraded: for cert in ungraded:
# grade the student # grade the student
grade = grades.grade(cert.user, request, course) grade = grades.grade(cert.user, course)
print "grading {0} - {1}".format(cert.user, grade['percent']) print "grading {0} - {1}".format(cert.user, grade['percent'])
cert.grade = grade['percent'] cert.grade = grade['percent']
if not options['noop']: if not options['noop']:
......
...@@ -257,7 +257,7 @@ class XQueueCertInterface(object): ...@@ -257,7 +257,7 @@ class XQueueCertInterface(object):
self.request.session = {} self.request.session = {}
is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists() is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists()
grade = grades.grade(student, self.request, course) grade = grades.grade(student, course)
enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id) enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id)
mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES
user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student) user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student)
......
...@@ -13,7 +13,7 @@ from badges.events.course_complete import get_completion_badge ...@@ -13,7 +13,7 @@ from badges.events.course_complete import get_completion_badge
from badges.models import BadgeAssertion from badges.models import BadgeAssertion
from badges.tests.factories import BadgeAssertionFactory, CourseCompleteImageConfigurationFactory from badges.tests.factories import BadgeAssertionFactory, CourseCompleteImageConfigurationFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls, ItemFactory
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
from certificates.management.commands import resubmit_error_certificates, regenerate_user, ungenerated_certs from certificates.management.commands import resubmit_error_certificates, regenerate_user, ungenerated_certs
from certificates.models import GeneratedCertificate, CertificateStatuses from certificates.models import GeneratedCertificate, CertificateStatuses
...@@ -33,6 +33,9 @@ class CertificateManagementTest(ModuleStoreTestCase): ...@@ -33,6 +33,9 @@ class CertificateManagementTest(ModuleStoreTestCase):
CourseFactory.create() CourseFactory.create()
for __ in range(3) for __ in range(3)
] ]
for course in self.courses:
chapter = ItemFactory.create(parent_location=course.location)
ItemFactory.create(parent_location=chapter.location, category='sequential', graded=True)
CourseCompleteImageConfigurationFactory.create() CourseCompleteImageConfigurationFactory.create()
def _create_cert(self, course_key, user, status, mode=CourseMode.HONOR): def _create_cert(self, course_key, user, status, mode=CourseMode.HONOR):
......
...@@ -11,16 +11,21 @@ from collections import OrderedDict ...@@ -11,16 +11,21 @@ from collections import OrderedDict
from django.conf import settings from django.conf import settings
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.test.client import Client from django.test.client import Client, RequestFactory
from django.test.utils import override_settings from django.test.utils import override_settings
from course_modes.models import CourseMode from course_modes.models import CourseMode
from badges.events.course_complete import get_completion_badge from lms.djangoapps.badges.events.course_complete import get_completion_badge
from badges.tests.factories import BadgeAssertionFactory, CourseCompleteImageConfigurationFactory, BadgeClassFactory from lms.djangoapps.badges.tests.factories import (
BadgeAssertionFactory,
CourseCompleteImageConfigurationFactory,
BadgeClassFactory,
)
from openedx.core.lib.tests.assertions.events import assert_event_matches from openedx.core.lib.tests.assertions.events import assert_event_matches
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
from student.roles import CourseStaffRole from student.roles import CourseStaffRole
from track.tests import EventTrackingTestCase from track.tests import EventTrackingTestCase
from util import organizations_helpers as organizations_api
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
...@@ -39,8 +44,6 @@ from certificates.tests.factories import ( ...@@ -39,8 +44,6 @@ from certificates.tests.factories import (
LinkedInAddToProfileConfigurationFactory, LinkedInAddToProfileConfigurationFactory,
GeneratedCertificateFactory, GeneratedCertificateFactory,
) )
from util import organizations_helpers as organizations_api
from django.test.client import RequestFactory
FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy()
FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True
...@@ -63,14 +66,12 @@ def _fake_is_request_in_microsite(): ...@@ -63,14 +66,12 @@ def _fake_is_request_in_microsite():
return True return True
@attr('shard_1') class CommonCertificatesTestCase(ModuleStoreTestCase):
@ddt.ddt
class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
""" """
Tests for the certificates web/html views Common setUp and utility methods for Certificate tests
""" """
def setUp(self): def setUp(self):
super(CertificatesViewsTests, self).setUp() super(CommonCertificatesTestCase, self).setUp()
self.client = Client() self.client = Client()
self.course = CourseFactory.create( self.course = CourseFactory.create(
org='testorg', number='run1', display_name='refundable course' org='testorg', number='run1', display_name='refundable course'
...@@ -85,7 +86,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ...@@ -85,7 +86,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
self.user.profile.save() self.user.profile.save()
self.client.login(username=self.user.username, password='foo') self.client.login(username=self.user.username, password='foo')
self.request = RequestFactory().request() self.request = RequestFactory().request()
self.linknedin_url = 'http://www.linkedin.com/profile/add?{params}' self.linkedin_url = 'http://www.linkedin.com/profile/add?{params}'
self.cert = GeneratedCertificateFactory.create( self.cert = GeneratedCertificateFactory.create(
user=self.user, user=self.user,
...@@ -168,6 +169,14 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ...@@ -168,6 +169,14 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
) )
template.save() template.save()
@attr('shard_1')
@ddt.ddt
class CertificatesViewsTests(CommonCertificatesTestCase):
"""
Tests for the certificates web/html views
"""
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
def test_linkedin_share_url(self): def test_linkedin_share_url(self):
""" """
...@@ -186,7 +195,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ...@@ -186,7 +195,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
('pfCertificationUrl', self.request.build_absolute_uri(test_url),), ('pfCertificationUrl', self.request.build_absolute_uri(test_url),),
]) ])
self.assertIn( self.assertIn(
self.linknedin_url.format(params=urlencode(params)), self.linkedin_url.format(params=urlencode(params)),
response.content response.content
) )
...@@ -209,7 +218,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ...@@ -209,7 +218,7 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
('pfCertificationUrl', 'http://' + settings.MICROSITE_TEST_HOSTNAME + test_url,), ('pfCertificationUrl', 'http://' + settings.MICROSITE_TEST_HOSTNAME + test_url,),
]) ])
self.assertIn( self.assertIn(
self.linknedin_url.format(params=urlencode(params)), self.linkedin_url.format(params=urlencode(params)),
response.content response.content
) )
...@@ -449,7 +458,6 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ...@@ -449,7 +458,6 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
"{partner_long_name}.".format( "{partner_long_name}.".format(
partner_short_name=short_org_name, partner_short_name=short_org_name,
partner_long_name=long_org_name, partner_long_name=long_org_name,
platform_name='Test Microsite'
), ),
response.content response.content
) )
...@@ -810,73 +818,6 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ...@@ -810,73 +818,6 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
with self.assertRaises(Exception): with self.assertRaises(Exception):
self.client.get(test_url) self.client.get(test_url)
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
def test_certificate_evidence_event_emitted(self):
self.client.logout()
self._add_course_certificates(count=1, signatory_count=2)
self.recreate_tracker()
test_url = get_certificate_url(
user_id=self.user.id,
course_id=unicode(self.course.id)
)
response = self.client.get(test_url)
self.assertEqual(response.status_code, 200)
actual_event = self.get_event()
self.assertEqual(actual_event['name'], 'edx.certificate.evidence_visited')
assert_event_matches(
{
'user_id': self.user.id,
'certificate_id': unicode(self.cert.verify_uuid),
'enrollment_mode': self.cert.mode,
'certificate_url': test_url,
'course_id': unicode(self.course.id),
'social_network': CertificateSocialNetworks.linkedin
},
actual_event['data']
)
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
def test_evidence_event_sent(self):
self._add_course_certificates(count=1, signatory_count=2)
cert_url = get_certificate_url(
user_id=self.user.id,
course_id=self.course_id
)
test_url = '{}?evidence_visit=1'.format(cert_url)
self.recreate_tracker()
badge_class = get_completion_badge(self.course_id, self.user)
assertion = BadgeAssertionFactory.create(
user=self.user, badge_class=badge_class,
backend='DummyBackend',
image_url='http://www.example.com/image.png',
assertion_url='http://www.example.com/assertion.json',
data={
'issuer': 'http://www.example.com/issuer.json',
}
)
response = self.client.get(test_url)
self.assertEqual(response.status_code, 200)
assert_event_matches(
{
'name': 'edx.badge.assertion.evidence_visited',
'data': {
'course_id': 'testorg/run1/refundable_course',
'assertion_id': assertion.id,
'badge_generator': u'DummyBackend',
'badge_name': u'refundable course',
'issuing_component': u'',
'badge_slug': u'testorgrun1refundable_course_honor_432f164',
'assertion_json_url': 'http://www.example.com/assertion.json',
'assertion_image_url': 'http://www.example.com/image.png',
'user_id': self.user.id,
'issuer': 'http://www.example.com/issuer.json',
'enrollment_mode': 'honor',
},
},
self.get_event()
)
@override_settings(FEATURES=FEATURES_WITH_CERTS_DISABLED) @override_settings(FEATURES=FEATURES_WITH_CERTS_DISABLED)
def test_request_certificate_without_passing(self): def test_request_certificate_without_passing(self):
self.cert.status = CertificateStatuses.unavailable self.cert.status = CertificateStatuses.unavailable
...@@ -1202,3 +1143,76 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ...@@ -1202,3 +1143,76 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase):
response, response,
configuration['microsites']['testmicrosite']['company_tos_url'], configuration['microsites']['testmicrosite']['company_tos_url'],
) )
@attr('shard_1')
class CertificateEventTests(CommonCertificatesTestCase, EventTrackingTestCase):
"""
Test events emitted by certificate handling.
"""
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
def test_certificate_evidence_event_emitted(self):
self.client.logout()
self._add_course_certificates(count=1, signatory_count=2)
self.recreate_tracker()
test_url = get_certificate_url(
user_id=self.user.id,
course_id=unicode(self.course.id)
)
response = self.client.get(test_url)
self.assertEqual(response.status_code, 200)
actual_event = self.get_event()
self.assertEqual(actual_event['name'], 'edx.certificate.evidence_visited')
assert_event_matches(
{
'user_id': self.user.id,
'certificate_id': unicode(self.cert.verify_uuid),
'enrollment_mode': self.cert.mode,
'certificate_url': test_url,
'course_id': unicode(self.course.id),
'social_network': CertificateSocialNetworks.linkedin
},
actual_event['data']
)
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
def test_evidence_event_sent(self):
self._add_course_certificates(count=1, signatory_count=2)
cert_url = get_certificate_url(
user_id=self.user.id,
course_id=self.course_id
)
test_url = '{}?evidence_visit=1'.format(cert_url)
self.recreate_tracker()
badge_class = get_completion_badge(self.course_id, self.user)
assertion = BadgeAssertionFactory.create(
user=self.user, badge_class=badge_class,
backend='DummyBackend',
image_url='http://www.example.com/image.png',
assertion_url='http://www.example.com/assertion.json',
data={
'issuer': 'http://www.example.com/issuer.json',
}
)
response = self.client.get(test_url)
self.assertEqual(response.status_code, 200)
assert_event_matches(
{
'name': 'edx.badge.assertion.evidence_visited',
'data': {
'course_id': 'testorg/run1/refundable_course',
'assertion_id': assertion.id,
'badge_generator': u'DummyBackend',
'badge_name': u'refundable course',
'issuing_component': u'',
'badge_slug': u'testorgrun1refundable_course_honor_432f164',
'assertion_json_url': 'http://www.example.com/assertion.json',
'assertion_image_url': 'http://www.example.com/image.png',
'user_id': self.user.id,
'issuer': 'http://www.example.com/issuer.json',
'enrollment_mode': 'honor',
},
},
self.get_event()
)
...@@ -17,10 +17,14 @@ class BlockSerializer(serializers.Serializer): # pylint: disable=abstract-metho ...@@ -17,10 +17,14 @@ class BlockSerializer(serializers.Serializer): # pylint: disable=abstract-metho
Get the field value requested. The field may be an XBlock field, a Get the field value requested. The field may be an XBlock field, a
transformer block field, or an entire tranformer block data dict. transformer block field, or an entire tranformer block data dict.
""" """
value = None
if transformer is None: if transformer is None:
value = self.context['block_structure'].get_xblock_field(block_key, field_name) value = self.context['block_structure'].get_xblock_field(block_key, field_name)
elif field_name is None: elif field_name is None:
value = self.context['block_structure'].get_transformer_block_data(block_key, transformer) try:
value = self.context['block_structure'].get_transformer_block_data(block_key, transformer).fields
except KeyError:
pass
else: else:
value = self.context['block_structure'].get_transformer_block_field(block_key, transformer, field_name) value = self.context['block_structure'].get_transformer_block_field(block_key, transformer, field_name)
......
...@@ -4,7 +4,6 @@ Tests for Blocks api.py ...@@ -4,7 +4,6 @@ Tests for Blocks api.py
from django.test.client import RequestFactory from django.test.client import RequestFactory
from openedx.core.djangoapps.content.block_structure.tests.helpers import EnableTransformerRegistryMixin
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
...@@ -13,7 +12,7 @@ from xmodule.modulestore.tests.factories import SampleCourseFactory ...@@ -13,7 +12,7 @@ from xmodule.modulestore.tests.factories import SampleCourseFactory
from ..api import get_blocks from ..api import get_blocks
class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): class TestGetBlocks(SharedModuleStoreTestCase):
""" """
Tests for the get_blocks function Tests for the get_blocks function
""" """
......
...@@ -7,7 +7,6 @@ from urllib import urlencode ...@@ -7,7 +7,6 @@ from urllib import urlencode
from rest_framework.exceptions import PermissionDenied from rest_framework.exceptions import PermissionDenied
from opaque_keys.edx.locator import CourseLocator from opaque_keys.edx.locator import CourseLocator
from openedx.core.djangoapps.content.block_structure.tests.helpers import EnableTransformerRegistryMixin
from openedx.core.djangoapps.util.test_forms import FormTestMixin from openedx.core.djangoapps.util.test_forms import FormTestMixin
from student.models import CourseEnrollment from student.models import CourseEnrollment
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
...@@ -18,7 +17,7 @@ from ..forms import BlockListGetForm ...@@ -18,7 +17,7 @@ from ..forms import BlockListGetForm
@ddt.ddt @ddt.ddt
class TestBlockListGetForm(EnableTransformerRegistryMixin, FormTestMixin, SharedModuleStoreTestCase): class TestBlockListGetForm(FormTestMixin, SharedModuleStoreTestCase):
""" """
Tests for BlockListGetForm Tests for BlockListGetForm
""" """
......
...@@ -3,7 +3,6 @@ Tests for Course Blocks serializers ...@@ -3,7 +3,6 @@ Tests for Course Blocks serializers
""" """
from mock import MagicMock from mock import MagicMock
from openedx.core.djangoapps.content.block_structure.tests.helpers import EnableTransformerRegistryMixin
from openedx.core.lib.block_structure.transformers import BlockStructureTransformers from openedx.core.lib.block_structure.transformers import BlockStructureTransformers
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
...@@ -17,7 +16,7 @@ from ..serializers import BlockSerializer, BlockDictSerializer ...@@ -17,7 +16,7 @@ from ..serializers import BlockSerializer, BlockDictSerializer
from .helpers import deserialize_usage_key from .helpers import deserialize_usage_key
class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): class TestBlockSerializerBase(SharedModuleStoreTestCase):
""" """
Base class for testing BlockSerializer and BlockDictSerializer Base class for testing BlockSerializer and BlockDictSerializer
""" """
...@@ -42,10 +41,11 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT ...@@ -42,10 +41,11 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT
block_types_to_count=['video'], block_types_to_count=['video'],
requested_student_view_data=['video'], requested_student_view_data=['video'],
) )
self.transformers = BlockStructureTransformers(COURSE_BLOCK_ACCESS_TRANSFORMERS + [blocks_api_transformer])
self.block_structure = get_course_blocks( self.block_structure = get_course_blocks(
self.user, self.user,
self.course.location, self.course.location,
BlockStructureTransformers(COURSE_BLOCK_ACCESS_TRANSFORMERS + [blocks_api_transformer]), self.transformers,
) )
self.serializer_context = { self.serializer_context = {
'request': MagicMock(), 'request': MagicMock(),
...@@ -93,7 +93,7 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT ...@@ -93,7 +93,7 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT
{ {
'id', 'type', 'lms_web_url', 'student_view_url', 'id', 'type', 'lms_web_url', 'student_view_url',
'display_name', 'graded', 'display_name', 'graded',
'block_counts', 'student_view_multi_device', 'student_view_multi_device',
'lti_url', 'lti_url',
'visible_to_staff_only', 'visible_to_staff_only',
}, },
...@@ -109,6 +109,13 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT ...@@ -109,6 +109,13 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT
self.assertIn('student_view_multi_device', serialized_block) self.assertIn('student_view_multi_device', serialized_block)
self.assertTrue(serialized_block['student_view_multi_device']) self.assertTrue(serialized_block['student_view_multi_device'])
# chapters with video should have block_counts
if serialized_block['type'] == 'chapter':
if serialized_block['display_name'] not in ('poll_test', 'handout_container'):
self.assertIn('block_counts', serialized_block)
else:
self.assertNotIn('block_counts', serialized_block)
def create_staff_context(self): def create_staff_context(self):
""" """
Create staff user and course blocks accessible by that user Create staff user and course blocks accessible by that user
...@@ -120,7 +127,7 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT ...@@ -120,7 +127,7 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT
block_structure = get_course_blocks( block_structure = get_course_blocks(
staff_user, staff_user,
self.course.location, self.course.location,
BlockStructureTransformers(COURSE_BLOCK_ACCESS_TRANSFORMERS), self.transformers,
) )
return { return {
'request': MagicMock(), 'request': MagicMock(),
...@@ -157,12 +164,14 @@ class TestBlockSerializer(TestBlockSerializerBase): ...@@ -157,12 +164,14 @@ class TestBlockSerializer(TestBlockSerializerBase):
serializer = self.create_serializer() serializer = self.create_serializer()
for serialized_block in serializer.data: for serialized_block in serializer.data:
self.assert_basic_block(serialized_block['id'], serialized_block) self.assert_basic_block(serialized_block['id'], serialized_block)
self.assertEquals(len(serializer.data), 28)
def test_additional_requested_fields(self): def test_additional_requested_fields(self):
self.add_additional_requested_fields() self.add_additional_requested_fields()
serializer = self.create_serializer() serializer = self.create_serializer()
for serialized_block in serializer.data: for serialized_block in serializer.data:
self.assert_extended_block(serialized_block) self.assert_extended_block(serialized_block)
self.assertEquals(len(serializer.data), 28)
def test_staff_fields(self): def test_staff_fields(self):
""" """
...@@ -174,6 +183,7 @@ class TestBlockSerializer(TestBlockSerializerBase): ...@@ -174,6 +183,7 @@ class TestBlockSerializer(TestBlockSerializerBase):
for serialized_block in serializer.data: for serialized_block in serializer.data:
self.assert_extended_block(serialized_block) self.assert_extended_block(serialized_block)
self.assert_staff_fields(serialized_block) self.assert_staff_fields(serialized_block)
self.assertEquals(len(serializer.data), 29)
class TestBlockDictSerializer(TestBlockSerializerBase): class TestBlockDictSerializer(TestBlockSerializerBase):
...@@ -201,12 +211,14 @@ class TestBlockDictSerializer(TestBlockSerializerBase): ...@@ -201,12 +211,14 @@ class TestBlockDictSerializer(TestBlockSerializerBase):
for block_key_string, serialized_block in serializer.data['blocks'].iteritems(): for block_key_string, serialized_block in serializer.data['blocks'].iteritems():
self.assertEquals(serialized_block['id'], block_key_string) self.assertEquals(serialized_block['id'], block_key_string)
self.assert_basic_block(block_key_string, serialized_block) self.assert_basic_block(block_key_string, serialized_block)
self.assertEquals(len(serializer.data['blocks']), 28)
def test_additional_requested_fields(self): def test_additional_requested_fields(self):
self.add_additional_requested_fields() self.add_additional_requested_fields()
serializer = self.create_serializer() serializer = self.create_serializer()
for serialized_block in serializer.data['blocks'].itervalues(): for serialized_block in serializer.data['blocks'].itervalues():
self.assert_extended_block(serialized_block) self.assert_extended_block(serialized_block)
self.assertEquals(len(serializer.data['blocks']), 28)
def test_staff_fields(self): def test_staff_fields(self):
""" """
...@@ -218,3 +230,4 @@ class TestBlockDictSerializer(TestBlockSerializerBase): ...@@ -218,3 +230,4 @@ class TestBlockDictSerializer(TestBlockSerializerBase):
for serialized_block in serializer.data['blocks'].itervalues(): for serialized_block in serializer.data['blocks'].itervalues():
self.assert_extended_block(serialized_block) self.assert_extended_block(serialized_block)
self.assert_staff_fields(serialized_block) self.assert_staff_fields(serialized_block)
self.assertEquals(len(serializer.data['blocks']), 29)
...@@ -8,7 +8,6 @@ from urllib import urlencode ...@@ -8,7 +8,6 @@ from urllib import urlencode
from urlparse import urlunparse from urlparse import urlunparse
from opaque_keys.edx.locator import CourseLocator from opaque_keys.edx.locator import CourseLocator
from openedx.core.djangoapps.content.block_structure.tests.helpers import EnableTransformerRegistryMixin
from student.models import CourseEnrollment from student.models import CourseEnrollment
from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
...@@ -17,7 +16,7 @@ from xmodule.modulestore.tests.factories import ToyCourseFactory ...@@ -17,7 +16,7 @@ from xmodule.modulestore.tests.factories import ToyCourseFactory
from .helpers import deserialize_usage_key from .helpers import deserialize_usage_key
class TestBlocksView(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): class TestBlocksView(SharedModuleStoreTestCase):
""" """
Test class for BlocksView Test class for BlocksView
""" """
......
...@@ -38,13 +38,13 @@ class TestBlockCountsTransformer(ModuleStoreTestCase): ...@@ -38,13 +38,13 @@ class TestBlockCountsTransformer(ModuleStoreTestCase):
) )
# verify count of chapters # verify count of chapters
self.assertEquals(block_counts_for_course['chapter'], 2) self.assertEquals(block_counts_for_course.chapter, 2)
# verify count of problems # verify count of problems
self.assertEquals(block_counts_for_course['problem'], 6) self.assertEquals(block_counts_for_course.problem, 6)
self.assertEquals(block_counts_for_chapter_x['problem'], 3) self.assertEquals(block_counts_for_chapter_x.problem, 3)
# verify other block types are not counted # verify other block types are not counted
for block_type in ['course', 'html', 'video']: for block_type in ['course', 'html', 'video']:
self.assertNotIn(block_type, block_counts_for_course) self.assertFalse(hasattr(block_counts_for_course, block_type))
self.assertNotIn(block_type, block_counts_for_chapter_x) self.assertFalse(hasattr(block_counts_for_chapter_x, block_type))
...@@ -15,8 +15,6 @@ class TestGenerateCourseBlocks(ModuleStoreTestCase): ...@@ -15,8 +15,6 @@ class TestGenerateCourseBlocks(ModuleStoreTestCase):
""" """
Tests generate course blocks management command. Tests generate course blocks management command.
""" """
ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
def setUp(self): def setUp(self):
""" """
Create courses in modulestore. Create courses in modulestore.
......
...@@ -36,8 +36,6 @@ class CourseStructureTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase) ...@@ -36,8 +36,6 @@ class CourseStructureTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase)
""" """
Helper for test cases that need to build course structures. Helper for test cases that need to build course structures.
""" """
ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
def setUp(self): def setUp(self):
""" """
Create users. Create users.
......
...@@ -5,30 +5,33 @@ import json ...@@ -5,30 +5,33 @@ import json
import logging import logging
import random import random
from collections import defaultdict from collections import defaultdict
from functools import partial
import dogstats_wrapper as dog_stats_api import dogstats_wrapper as dog_stats_api
from course_blocks.api import get_course_blocks
from courseware import courses
from django.conf import settings from django.conf import settings
from django.core.cache import cache from django.core.cache import cache
from django.test.client import RequestFactory from django.test.client import RequestFactory
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import BlockUsageLocator from opaque_keys.edx.locator import BlockUsageLocator
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
from openedx.core.lib.cache_utils import memoized
from openedx.core.lib.gating import api as gating_api from openedx.core.lib.gating import api as gating_api
from courseware import courses
from courseware.access import has_access
from courseware.model_data import FieldDataCache, ScoresClient from courseware.model_data import FieldDataCache, ScoresClient
from openedx.core.djangoapps.signals.signals import GRADES_UPDATED from openedx.core.djangoapps.signals.signals import GRADES_UPDATED
from student.models import anonymous_id_for_user from student.models import anonymous_id_for_user
from util.db import outer_atomic from util.db import outer_atomic
from util.module_utils import yield_dynamic_descriptor_descendants from util.module_utils import yield_dynamic_descriptor_descendants
from xmodule import graders from xblock.core import XBlock
from xmodule import graders, block_metadata_utils
from xmodule.graders import Score from xmodule.graders import Score
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
from .models import StudentModule from .models import StudentModule
from .module_render import get_module_for_descriptor from .module_render import get_module_for_descriptor
from .transformers.grades import GradesTransformer
log = logging.getLogger("edx.courseware") log = logging.getLogger("edx.courseware")
...@@ -141,7 +144,7 @@ class ProgressSummary(object): ...@@ -141,7 +144,7 @@ class ProgressSummary(object):
weighted_scores: a dictionary mapping module locations to weighted Score weighted_scores: a dictionary mapping module locations to weighted Score
objects. objects.
locations_to_children: a dictionary mapping module locations to their locations_to_children: a function mapping locations to their
direct descendants. direct descendants.
""" """
def __init__(self, chapters, weighted_scores, locations_to_children): def __init__(self, chapters, weighted_scores, locations_to_children):
...@@ -172,34 +175,93 @@ class ProgressSummary(object): ...@@ -172,34 +175,93 @@ class ProgressSummary(object):
return earned, possible return earned, possible
def descriptor_affects_grading(block_types_affecting_grading, descriptor): @memoized
def block_types_with_scores():
""" """
Returns True if the descriptor could have any impact on grading, else False. Returns the block types that could have a score.
Something might be a scored item if it is capable of storing a score Something might be a scored item if it is capable of storing a score
(has_score=True). We also have to include anything that can have children, (has_score=True). We also have to include anything that can have children,
since those children might have scores. We can avoid things like Videos, since those children might have scores. We can avoid things like Videos,
which have state but cannot ever impact someone's grade. which have state but cannot ever impact someone's grade.
""" """
return descriptor.location.block_type in block_types_affecting_grading return frozenset(
cat for (cat, xblock_class) in XBlock.load_classes() if (
getattr(xblock_class, 'has_score', False) or getattr(xblock_class, 'has_children', False)
)
)
def field_data_cache_for_grading(course, user): def possibly_scored(usage_key):
"""
Returns whether the given block could impact grading (i.e. scored, or has children).
""" """
Given a CourseDescriptor and User, create the FieldDataCache for grading. return usage_key.block_type in block_types_with_scores()
This will generate a FieldDataCache that only loads state for those things
that might possibly affect the grading process, and will ignore things like def grading_context_for_course(course):
Videos.
""" """
descriptor_filter = partial(descriptor_affects_grading, course.block_types_affecting_grading) Same as grading_context, but takes in a course object.
return FieldDataCache.cache_for_descriptor_descendents( """
course.id, course_structure = get_course_in_cache(course.id)
user, return grading_context(course_structure)
course,
depth=None,
descriptor_filter=descriptor_filter def grading_context(course_structure):
) """
This returns a dictionary with keys necessary for quickly grading
a student. They are used by grades.grade()
The grading context has two keys:
graded_sections - This contains the sections that are graded, as
well as all possible children modules that can affect the
grading. This allows some sections to be skipped if the student
hasn't seen any part of it.
The format is a dictionary keyed by section-type. The values are
arrays of dictionaries containing
"section_block" : The section block
"scored_descendant_keys" : An array of usage keys for blocks
could possibly be in the section, for any student
all_graded_blocks - This contains a list of all blocks that can
affect grading a student. This is used to efficiently fetch
all the xmodule state for a FieldDataCache without walking
the descriptor tree again.
"""
all_graded_blocks = []
all_graded_sections = defaultdict(list)
for chapter_key in course_structure.get_children(course_structure.root_block_usage_key):
for section_key in course_structure.get_children(chapter_key):
section = course_structure[section_key]
scored_descendants_of_section = [section]
if section.graded:
for descendant_key in course_structure.post_order_traversal(
filter_func=possibly_scored,
start_node=section_key,
):
scored_descendants_of_section.append(
course_structure[descendant_key],
)
# include only those blocks that have scores, not if they are just a parent
section_info = {
'section_block': section,
'scored_descendants': [
child for child in scored_descendants_of_section
if getattr(child, 'has_score', None)
]
}
section_format = getattr(section, 'format', '')
all_graded_sections[section_format].append(section_info)
all_graded_blocks.extend(scored_descendants_of_section)
return {
'all_graded_sections': all_graded_sections,
'all_graded_blocks': all_graded_blocks,
}
def answer_distributions(course_key): def answer_distributions(course_key):
...@@ -213,7 +275,7 @@ def answer_distributions(course_key): ...@@ -213,7 +275,7 @@ def answer_distributions(course_key):
entries for a given course with type="problem" and a grade that is not null. entries for a given course with type="problem" and a grade that is not null.
This means that we only count LoncapaProblems that people have submitted. This means that we only count LoncapaProblems that people have submitted.
Other types of items like ORA or sequences will not be collected. Empty Other types of items like ORA or sequences will not be collected. Empty
Loncapa problem state that gets created from runnig the progress page is Loncapa problem state that gets created from running the progress page is
also not counted. also not counted.
This method accesses the StudentModule table directly instead of using the This method accesses the StudentModule table directly instead of using the
...@@ -295,13 +357,13 @@ def answer_distributions(course_key): ...@@ -295,13 +357,13 @@ def answer_distributions(course_key):
return answer_counts return answer_counts
def grade(student, request, course, keep_raw_scores=False, field_data_cache=None, scores_client=None): def grade(student, course, keep_raw_scores=False):
""" """
Returns the grade of the student. Returns the grade of the student.
Also sends a signal to update the minimum grade requirement status. Also sends a signal to update the minimum grade requirement status.
""" """
grade_summary = _grade(student, request, course, keep_raw_scores, field_data_cache, scores_client) grade_summary = _grade(student, course, keep_raw_scores)
responses = GRADES_UPDATED.send_robust( responses = GRADES_UPDATED.send_robust(
sender=None, sender=None,
username=student.username, username=student.username,
...@@ -316,7 +378,7 @@ def grade(student, request, course, keep_raw_scores=False, field_data_cache=None ...@@ -316,7 +378,7 @@ def grade(student, request, course, keep_raw_scores=False, field_data_cache=None
return grade_summary return grade_summary
def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_client): def _grade(student, course, keep_raw_scores):
""" """
Unwrapped version of "grade" Unwrapped version of "grade"
...@@ -324,24 +386,18 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c ...@@ -324,24 +386,18 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c
output from the course grader, augmented with the final letter output from the course grader, augmented with the final letter
grade. The keys in the output are: grade. The keys in the output are:
course: a CourseDescriptor - course: a CourseDescriptor
- grade : A final letter grade.
- percent : The final percent for the class (rounded up).
- section_breakdown : A breakdown of each section that makes
up the grade. (For display)
- grade_breakdown : A breakdown of the major components that
make up the final grade. (For display)
- keep_raw_scores : if True, then value for key 'raw_scores' contains scores - keep_raw_scores : if True, then value for key 'raw_scores' contains scores
for every graded module for every graded module
More information on the format is in the docstring for CourseGrader. More information on the format is in the docstring for CourseGrader.
""" """
course_structure = get_course_blocks(student, course.location)
grading_context_result = grading_context(course_structure)
scorable_locations = [block.location for block in grading_context_result['all_graded_blocks']]
with outer_atomic(): with outer_atomic():
if field_data_cache is None: scores_client = ScoresClient.create_for_locations(course.id, student.id, scorable_locations)
field_data_cache = field_data_cache_for_grading(course, student)
if scores_client is None:
scores_client = ScoresClient.from_field_data_cache(field_data_cache)
# Dict of item_ids -> (earned, possible) point tuples. This *only* grabs # Dict of item_ids -> (earned, possible) point tuples. This *only* grabs
# scores that were registered with the submissions API, which for the moment # scores that were registered with the submissions API, which for the moment
...@@ -358,47 +414,69 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c ...@@ -358,47 +414,69 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c
) )
max_scores_cache = MaxScoresCache.create_for_course(course) max_scores_cache = MaxScoresCache.create_for_course(course)
# For the moment, we have to get scorable_locations from field_data_cache # For the moment, scores_client is ignorant of scorable_locations
# and not from scores_client, because scores_client is ignorant of things
# in the submissions API. As a further refactoring step, submissions should # in the submissions API. As a further refactoring step, submissions should
# be hidden behind the ScoresClient. # be hidden behind the ScoresClient.
max_scores_cache.fetch_from_remote(field_data_cache.scorable_locations) max_scores_cache.fetch_from_remote(scorable_locations)
grading_context = course.grading_context totaled_scores, raw_scores = _calculate_totaled_scores(
raw_scores = [] student, grading_context_result, max_scores_cache, submissions_scores, scores_client, keep_raw_scores
)
with outer_atomic():
# Grading policy might be overriden by a CCX, need to reset it
course.set_grading_policy(course.grading_policy)
grade_summary = course.grader.grade(totaled_scores, generate_random_scores=settings.GENERATE_PROFILE_SCORES)
# We round the grade here, to make sure that the grade is a whole percentage and
# doesn't get displayed differently than it gets grades
grade_summary['percent'] = round(grade_summary['percent'] * 100 + 0.05) / 100
letter_grade = grade_for_percentage(course.grade_cutoffs, grade_summary['percent'])
grade_summary['grade'] = letter_grade
grade_summary['totaled_scores'] = totaled_scores # make this available, eg for instructor download & debugging
if keep_raw_scores:
# way to get all RAW scores out to instructor
# so grader can be double-checked
grade_summary['raw_scores'] = raw_scores
max_scores_cache.push_to_remote()
return grade_summary
def _calculate_totaled_scores(
student,
grading_context_result,
max_scores_cache,
submissions_scores,
scores_client,
keep_raw_scores,
):
"""
Returns the totaled scores, which can be passed to the grader.
"""
raw_scores = []
totaled_scores = {} totaled_scores = {}
# This next complicated loop is just to collect the totaled_scores, which is for section_format, sections in grading_context_result['all_graded_sections'].iteritems():
# passed to the grader
for section_format, sections in grading_context['graded_sections'].iteritems():
format_scores = [] format_scores = []
for section in sections: for section_info in sections:
section_descriptor = section['section_descriptor'] section = section_info['section_block']
section_name = section_descriptor.display_name_with_default_escaped section_name = block_metadata_utils.display_name_with_default(section)
with outer_atomic(): with outer_atomic():
# some problems have state that is updated independently of interaction # Check to
# with the LMS, so they need to always be scored. (E.g. combinedopenended ORA1)
# TODO This block is causing extra savepoints to be fired that are empty because no queries are executed
# during the loop. When refactoring this code please keep this outer_atomic call in mind and ensure we
# are not making unnecessary database queries.
should_grade_section = any(
descriptor.always_recalculate_grades for descriptor in section['xmoduledescriptors']
)
# If there are no problems that always have to be regraded, check to
# see if any of our locations are in the scores from the submissions # see if any of our locations are in the scores from the submissions
# API. If scores exist, we have to calculate grades for this section. # API. If scores exist, we have to calculate grades for this section.
if not should_grade_section: should_grade_section = any(
should_grade_section = any( unicode(descendant.location) in submissions_scores
descriptor.location.to_deprecated_string() in submissions_scores for descendant in section_info['scored_descendants']
for descriptor in section['xmoduledescriptors'] )
)
if not should_grade_section: if not should_grade_section:
should_grade_section = any( should_grade_section = any(
descriptor.location in scores_client descendant.location in scores_client
for descriptor in section['xmoduledescriptors'] for descendant in section_info['scored_descendants']
) )
# If we haven't seen a single problem in the section, we don't have # If we haven't seen a single problem in the section, we don't have
...@@ -406,26 +484,11 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c ...@@ -406,26 +484,11 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c
if should_grade_section: if should_grade_section:
scores = [] scores = []
def create_module(descriptor): for descendant in section_info['scored_descendants']:
'''creates an XModule instance given a descriptor'''
# TODO: We need the request to pass into here. If we could forego that, our arguments
# would be simpler
return get_module_for_descriptor(
student, request, descriptor, field_data_cache, course.id, course=course
)
descendants = yield_dynamic_descriptor_descendants(section_descriptor, student.id, create_module)
for module_descriptor in descendants:
user_access = has_access(
student, 'load', module_descriptor, module_descriptor.location.course_key
)
if not user_access:
continue
(correct, total) = get_score( (correct, total) = get_score(
student, student,
module_descriptor, descendant,
create_module,
scores_client, scores_client,
submissions_scores, submissions_scores,
max_scores_cache, max_scores_cache,
...@@ -433,13 +496,13 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c ...@@ -433,13 +496,13 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c
if correct is None and total is None: if correct is None and total is None:
continue continue
if settings.GENERATE_PROFILE_SCORES: # for debugging! if settings.GENERATE_PROFILE_SCORES: # for debugging!
if total > 1: if total > 1:
correct = random.randrange(max(total - 2, 1), total + 1) correct = random.randrange(max(total - 2, 1), total + 1)
else: else:
correct = total correct = total
graded = module_descriptor.graded graded = descendant.graded
if not total > 0: if not total > 0:
# We simply cannot grade a problem that is 12/0, because we might need it as a percentage # We simply cannot grade a problem that is 12/0, because we might need it as a percentage
graded = False graded = False
...@@ -449,8 +512,8 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c ...@@ -449,8 +512,8 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c
correct, correct,
total, total,
graded, graded,
module_descriptor.display_name_with_default_escaped, block_metadata_utils.display_name_with_default_escaped(descendant),
module_descriptor.location descendant.location
) )
) )
...@@ -460,37 +523,18 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c ...@@ -460,37 +523,18 @@ def _grade(student, request, course, keep_raw_scores, field_data_cache, scores_c
else: else:
graded_total = Score(0.0, 1.0, True, section_name, None) graded_total = Score(0.0, 1.0, True, section_name, None)
#Add the graded total to totaled_scores # Add the graded total to totaled_scores
if graded_total.possible > 0: if graded_total.possible > 0:
format_scores.append(graded_total) format_scores.append(graded_total)
else: else:
log.info( log.info(
"Unable to grade a section with a total possible score of zero. " + "Unable to grade a section with a total possible score of zero. " +
str(section_descriptor.location) str(section.location)
) )
totaled_scores[section_format] = format_scores totaled_scores[section_format] = format_scores
with outer_atomic(): return totaled_scores, raw_scores
# Grading policy might be overriden by a CCX, need to reset it
course.set_grading_policy(course.grading_policy)
grade_summary = course.grader.grade(totaled_scores, generate_random_scores=settings.GENERATE_PROFILE_SCORES)
# We round the grade here, to make sure that the grade is an whole percentage and
# doesn't get displayed differently than it gets grades
grade_summary['percent'] = round(grade_summary['percent'] * 100 + 0.05) / 100
letter_grade = grade_for_percentage(course.grade_cutoffs, grade_summary['percent'])
grade_summary['grade'] = letter_grade
grade_summary['totaled_scores'] = totaled_scores # make this available, eg for instructor download & debugging
if keep_raw_scores:
# way to get all RAW scores out to instructor
# so grader can be double-checked
grade_summary['raw_scores'] = raw_scores
max_scores_cache.push_to_remote()
return grade_summary
def grade_for_percentage(grade_cutoffs, percentage): def grade_for_percentage(grade_cutoffs, percentage):
...@@ -515,30 +559,27 @@ def grade_for_percentage(grade_cutoffs, percentage): ...@@ -515,30 +559,27 @@ def grade_for_percentage(grade_cutoffs, percentage):
return letter_grade return letter_grade
def progress_summary(student, request, course, field_data_cache=None, scores_client=None): def progress_summary(student, course):
""" """
Returns progress summary for all chapters in the course. Returns progress summary for all chapters in the course.
""" """
progress = _progress_summary(student, request, course, field_data_cache, scores_client)
progress = _progress_summary(student, course)
if progress: if progress:
return progress.chapters return progress.chapters
else: else:
return None return None
def get_weighted_scores(student, course, field_data_cache=None, scores_client=None): def get_weighted_scores(student, course):
""" """
Uses the _progress_summary method to return a ProgressSummmary object Uses the _progress_summary method to return a ProgressSummary object
containing details of a students weighted scores for the course. containing details of a students weighted scores for the course.
""" """
request = _get_mock_request(student) return _progress_summary(student, course)
return _progress_summary(student, request, course, field_data_cache, scores_client)
# TODO: This method is not very good. It was written in the old course style and def _progress_summary(student, course):
# then converted over and performance is not good. Once the progress page is redesigned
# to not have the progress summary this method should be deleted (so it won't be copied).
def _progress_summary(student, request, course, field_data_cache=None, scores_client=None):
""" """
Unwrapped version of "progress_summary". Unwrapped version of "progress_summary".
...@@ -550,28 +591,20 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl ...@@ -550,28 +591,20 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl
each containing an array of scores. This contains information for graded and each containing an array of scores. This contains information for graded and
ungraded problems, and is good for displaying a course summary with due dates, ungraded problems, and is good for displaying a course summary with due dates,
etc. etc.
- None if the student does not have access to load the course module.
Arguments: Arguments:
student: A User object for the student to grade student: A User object for the student to grade
course: A Descriptor containing the course to grade course: A Descriptor containing the course to grade
If the student does not have access to load the course module, this function
will return None.
""" """
with outer_atomic(): course_structure = get_course_blocks(student, course.location)
if field_data_cache is None: if not len(course_structure):
field_data_cache = field_data_cache_for_grading(course, student) return None
if scores_client is None: scorable_locations = [block_key for block_key in course_structure if possibly_scored(block_key)]
scores_client = ScoresClient.from_field_data_cache(field_data_cache)
course_module = get_module_for_descriptor(
student, request, course, field_data_cache, course.id, course=course
)
if not course_module:
return None
course_module = getattr(course_module, '_x_module', course_module) with outer_atomic():
scores_client = ScoresClient.create_for_locations(course.id, student.id, scorable_locations)
# We need to import this here to avoid a circular dependency of the form: # We need to import this here to avoid a circular dependency of the form:
# XBlock --> submissions --> Django Rest Framework error strings --> # XBlock --> submissions --> Django Rest Framework error strings -->
...@@ -579,93 +612,83 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl ...@@ -579,93 +612,83 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl
from submissions import api as sub_api # installed from the edx-submissions repository from submissions import api as sub_api # installed from the edx-submissions repository
with outer_atomic(): with outer_atomic():
submissions_scores = sub_api.get_scores( submissions_scores = sub_api.get_scores(
course.id.to_deprecated_string(), anonymous_id_for_user(student, course.id) unicode(course.id), anonymous_id_for_user(student, course.id)
) )
max_scores_cache = MaxScoresCache.create_for_course(course) max_scores_cache = MaxScoresCache.create_for_course(course)
# For the moment, we have to get scorable_locations from field_data_cache # For the moment, scores_client is ignorant of scorable_locations
# and not from scores_client, because scores_client is ignorant of things
# in the submissions API. As a further refactoring step, submissions should # in the submissions API. As a further refactoring step, submissions should
# be hidden behind the ScoresClient. # be hidden behind the ScoresClient.
max_scores_cache.fetch_from_remote(field_data_cache.scorable_locations) max_scores_cache.fetch_from_remote(scorable_locations)
# Check for gated content # Check for gated content
gated_content = gating_api.get_gated_content(course, student) gated_content = gating_api.get_gated_content(course, student)
chapters = [] chapters = []
locations_to_children = defaultdict(list)
locations_to_weighted_scores = {} locations_to_weighted_scores = {}
# Don't include chapters that aren't displayable (e.g. due to error)
for chapter_module in course_module.get_display_items():
# Skip if the chapter is hidden
if chapter_module.hide_from_toc:
continue
for chapter_key in course_structure.get_children(course_structure.root_block_usage_key):
chapter = course_structure[chapter_key]
sections = [] sections = []
for section_module in chapter_module.get_display_items(): for section_key in course_structure.get_children(chapter_key):
# Skip if the section is hidden if unicode(section_key) in gated_content:
with outer_atomic(): continue
if section_module.hide_from_toc or unicode(section_module.location) in gated_content:
section = course_structure[section_key]
graded = getattr(section, 'graded', False)
scores = []
for descendant_key in course_structure.post_order_traversal(
filter_func=possibly_scored,
start_node=section_key,
):
descendant = course_structure[descendant_key]
(correct, total) = get_score(
student,
descendant,
scores_client,
submissions_scores,
max_scores_cache,
)
if correct is None and total is None:
continue continue
graded = section_module.graded weighted_location_score = Score(
scores = [] correct,
total,
module_creator = section_module.xmodule_runtime.get_module graded,
block_metadata_utils.display_name_with_default_escaped(descendant),
for module_descriptor in yield_dynamic_descriptor_descendants( descendant.location
section_module, student.id, module_creator )
):
location_parent = module_descriptor.parent.replace(version=None, branch=None)
location_to_save = module_descriptor.location.replace(version=None, branch=None)
locations_to_children[location_parent].append(location_to_save)
(correct, total) = get_score(
student,
module_descriptor,
module_creator,
scores_client,
submissions_scores,
max_scores_cache,
)
if correct is None and total is None:
continue
weighted_location_score = Score(
correct,
total,
graded,
module_descriptor.display_name_with_default_escaped,
module_descriptor.location
)
scores.append(weighted_location_score) scores.append(weighted_location_score)
locations_to_weighted_scores[location_to_save] = weighted_location_score locations_to_weighted_scores[descendant.location] = weighted_location_score
scores.reverse() escaped_section_name = block_metadata_utils.display_name_with_default_escaped(section)
section_total, _ = graders.aggregate_scores( section_total, _ = graders.aggregate_scores(scores, escaped_section_name)
scores, section_module.display_name_with_default_escaped)
module_format = section_module.format if section_module.format is not None else '' sections.append({
sections.append({ 'display_name': escaped_section_name,
'display_name': section_module.display_name_with_default_escaped, 'url_name': block_metadata_utils.url_name_for_block(section),
'url_name': section_module.url_name, 'scores': scores,
'scores': scores, 'section_total': section_total,
'section_total': section_total, 'format': getattr(section, 'format', ''),
'format': module_format, 'due': getattr(section, 'due', None),
'due': section_module.due, 'graded': graded,
'graded': graded, })
})
chapters.append({ chapters.append({
'course': course.display_name_with_default_escaped, 'course': course.display_name_with_default_escaped,
'display_name': chapter_module.display_name_with_default_escaped, 'display_name': block_metadata_utils.display_name_with_default_escaped(chapter),
'url_name': chapter_module.url_name, 'url_name': block_metadata_utils.url_name_for_block(chapter),
'sections': sections 'sections': sections
}) })
max_scores_cache.push_to_remote() max_scores_cache.push_to_remote()
return ProgressSummary(chapters, locations_to_weighted_scores, locations_to_children) return ProgressSummary(chapters, locations_to_weighted_scores, course_structure.get_children)
def weighted_score(raw_correct, raw_total, weight): def weighted_score(raw_correct, raw_total, weight):
...@@ -676,7 +699,7 @@ def weighted_score(raw_correct, raw_total, weight): ...@@ -676,7 +699,7 @@ def weighted_score(raw_correct, raw_total, weight):
return (float(raw_correct) * weight / raw_total, float(weight)) return (float(raw_correct) * weight / raw_total, float(weight))
def get_score(user, problem_descriptor, module_creator, scores_client, submissions_scores_cache, max_scores_cache): def get_score(user, block, scores_client, submissions_scores_cache, max_scores_cache):
""" """
Return the score for a user on a problem, as a tuple (correct, total). Return the score for a user on a problem, as a tuple (correct, total).
e.g. (5,7) if you got 5 out of 7 points. e.g. (5,7) if you got 5 out of 7 points.
...@@ -685,10 +708,8 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio ...@@ -685,10 +708,8 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio
None). None).
user: a Student object user: a Student object
problem_descriptor: an XModuleDescriptor block: a BlockStructure's BlockData object
scores_client: an initialized ScoresClient scores_client: an initialized ScoresClient
module_creator: a function that takes a descriptor, and returns the corresponding XModule for this user.
Can return None if user doesn't have access, or if something else went wrong.
submissions_scores_cache: A dict of location names to (earned, possible) point tuples. submissions_scores_cache: A dict of location names to (earned, possible) point tuples.
If an entry is found in this cache, it takes precedence. If an entry is found in this cache, it takes precedence.
max_scores_cache: a MaxScoresCache max_scores_cache: a MaxScoresCache
...@@ -698,23 +719,11 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio ...@@ -698,23 +719,11 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio
if not user.is_authenticated(): if not user.is_authenticated():
return (None, None) return (None, None)
location_url = problem_descriptor.location.to_deprecated_string() location_url = unicode(block.location)
if location_url in submissions_scores_cache: if location_url in submissions_scores_cache:
return submissions_scores_cache[location_url] return submissions_scores_cache[location_url]
# some problems have state that is updated independently of interaction if not getattr(block, 'has_score', False):
# with the LMS, so they need to always be scored. (E.g. combinedopenended ORA1.)
if problem_descriptor.always_recalculate_grades:
problem = module_creator(problem_descriptor)
if problem is None:
return (None, None)
score = problem.get_score()
if score is not None:
return (score['score'], score['total'])
else:
return (None, None)
if not problem_descriptor.has_score:
# These are not problems, and do not have a score # These are not problems, and do not have a score
return (None, None) return (None, None)
...@@ -723,8 +732,8 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio ...@@ -723,8 +732,8 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio
# value. This is important for cases where a student might have seen an # value. This is important for cases where a student might have seen an
# older version of the problem -- they're still graded on what was possible # older version of the problem -- they're still graded on what was possible
# when they tried the problem, not what it's worth now. # when they tried the problem, not what it's worth now.
score = scores_client.get(problem_descriptor.location) score = scores_client.get(block.location)
cached_max_score = max_scores_cache.get(problem_descriptor.location) cached_max_score = max_scores_cache.get(block.location)
if score and score.total is not None: if score and score.total is not None:
# We have a valid score, just use it. # We have a valid score, just use it.
correct = score.correct if score.correct is not None else 0.0 correct = score.correct if score.correct is not None else 0.0
...@@ -736,25 +745,18 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio ...@@ -736,25 +745,18 @@ def get_score(user, problem_descriptor, module_creator, scores_client, submissio
total = cached_max_score total = cached_max_score
else: else:
# This means we don't have a valid score entry and we don't have a # This means we don't have a valid score entry and we don't have a
# cached_max_score on hand. We know they've earned 0.0 points on this, # cached_max_score on hand. We know they've earned 0.0 points on this.
# but we need to instantiate the module (i.e. load student state) in
# order to find out how much it was worth.
problem = module_creator(problem_descriptor)
if problem is None:
return (None, None)
correct = 0.0 correct = 0.0
total = problem.max_score() total = block.transformer_data[GradesTransformer].max_score
# Problem may be an error module (if something in the problem builder failed) # Problem may be an error module (if something in the problem builder failed)
# In which case total might be None # In which case total might be None
if total is None: if total is None:
return (None, None) return (None, None)
else: else:
# add location to the max score cache max_scores_cache.set(block.location, total)
max_scores_cache.set(problem_descriptor.location, total)
return weighted_score(correct, total, problem_descriptor.weight) return weighted_score(correct, total, block.weight)
def iterate_grades_for(course_or_id, students, keep_raw_scores=False): def iterate_grades_for(course_or_id, students, keep_raw_scores=False):
...@@ -783,13 +785,7 @@ def iterate_grades_for(course_or_id, students, keep_raw_scores=False): ...@@ -783,13 +785,7 @@ def iterate_grades_for(course_or_id, students, keep_raw_scores=False):
for student in students: for student in students:
with dog_stats_api.timer('lms.grades.iterate_grades_for', tags=[u'action:{}'.format(course.id)]): with dog_stats_api.timer('lms.grades.iterate_grades_for', tags=[u'action:{}'.format(course.id)]):
try: try:
request = _get_mock_request(student) gradeset = grade(student, course, keep_raw_scores)
# Grading calls problem rendering, which calls masquerading,
# which checks session vars -- thus the empty session dict below.
# It's not pretty, but untangling that is currently beyond the
# scope of this feature.
request.session = {}
gradeset = grade(student, request, course, keep_raw_scores)
yield student, gradeset, "" yield student, gradeset, ""
except Exception as exc: # pylint: disable=broad-except except Exception as exc: # pylint: disable=broad-except
# Keep marching on even if this student couldn't be graded for # Keep marching on even if this student couldn't be graded for
......
...@@ -940,7 +940,6 @@ class ScoresClient(object): ...@@ -940,7 +940,6 @@ class ScoresClient(object):
Score = namedtuple('Score', 'correct total') Score = namedtuple('Score', 'correct total')
def __init__(self, course_key, user_id): def __init__(self, course_key, user_id):
"""Basic constructor. from_field_data_cache() is more appopriate for most uses."""
self.course_key = course_key self.course_key = course_key
self.user_id = user_id self.user_id = user_id
self._locations_to_scores = {} self._locations_to_scores = {}
...@@ -983,10 +982,10 @@ class ScoresClient(object): ...@@ -983,10 +982,10 @@ class ScoresClient(object):
return self._locations_to_scores.get(location.replace(version=None, branch=None)) return self._locations_to_scores.get(location.replace(version=None, branch=None))
@classmethod @classmethod
def from_field_data_cache(cls, fd_cache): def create_for_locations(cls, course_id, user_id, scorable_locations):
"""Create a ScoresClient from a populated FieldDataCache.""" """Create a ScoresClient with pre-fetched data for the given locations."""
client = cls(fd_cache.course_id, fd_cache.user.id) client = cls(course_id, user_id)
client.fetch_scores(fd_cache.scorable_locations) client.fetch_scores(scorable_locations)
return client return client
......
...@@ -82,9 +82,6 @@ class BaseTestXmodule(ModuleStoreTestCase): ...@@ -82,9 +82,6 @@ class BaseTestXmodule(ModuleStoreTestCase):
self.item_descriptor.xmodule_runtime = self.new_module_runtime() self.item_descriptor.xmodule_runtime = self.new_module_runtime()
#self.item_module = self.item_descriptor.xmodule_runtime.xmodule_instance
#self.item_module is None at this time
self.item_url = unicode(self.item_descriptor.location) self.item_url = unicode(self.item_descriptor.location)
def setup_course(self): def setup_course(self):
......
...@@ -6,7 +6,6 @@ from mock import Mock ...@@ -6,7 +6,6 @@ from mock import Mock
from . import BaseTestXmodule from . import BaseTestXmodule
from course_api.blocks.tests.helpers import deserialize_usage_key from course_api.blocks.tests.helpers import deserialize_usage_key
from courseware.module_render import get_module_for_descriptor_internal from courseware.module_render import get_module_for_descriptor_internal
from openedx.core.djangoapps.content.block_structure.tests.helpers import EnableTransformerRegistryMixin
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
from xmodule.discussion_module import DiscussionModule from xmodule.discussion_module import DiscussionModule
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
...@@ -16,7 +15,7 @@ from xmodule.modulestore.tests.factories import ToyCourseFactory, ItemFactory ...@@ -16,7 +15,7 @@ from xmodule.modulestore.tests.factories import ToyCourseFactory, ItemFactory
@ddt.ddt @ddt.ddt
class DiscussionModuleTest(BaseTestXmodule, EnableTransformerRegistryMixin, SharedModuleStoreTestCase): class DiscussionModuleTest(BaseTestXmodule, SharedModuleStoreTestCase):
"""Logic tests for Discussion Xmodule.""" """Logic tests for Discussion Xmodule."""
CATEGORY = "discussion" CATEGORY = "discussion"
......
...@@ -17,6 +17,7 @@ from ..field_overrides import ( ...@@ -17,6 +17,7 @@ from ..field_overrides import (
OverrideFieldData, OverrideFieldData,
OverrideModulestoreFieldData, OverrideModulestoreFieldData,
) )
from ..testutils import FieldOverrideTestMixin
TESTUSER = "testuser" TESTUSER = "testuser"
...@@ -128,15 +129,7 @@ class OverrideFieldDataTests(SharedModuleStoreTestCase): ...@@ -128,15 +129,7 @@ class OverrideFieldDataTests(SharedModuleStoreTestCase):
@override_settings( @override_settings(
MODULESTORE_FIELD_OVERRIDE_PROVIDERS=['courseware.tests.test_field_overrides.TestOverrideProvider'] MODULESTORE_FIELD_OVERRIDE_PROVIDERS=['courseware.tests.test_field_overrides.TestOverrideProvider']
) )
class OverrideModulestoreFieldDataTests(OverrideFieldDataTests): class OverrideModulestoreFieldDataTests(FieldOverrideTestMixin, OverrideFieldDataTests):
def setUp(self):
super(OverrideModulestoreFieldDataTests, self).setUp()
OverrideModulestoreFieldData.provider_classes = None
def tearDown(self):
super(OverrideModulestoreFieldDataTests, self).tearDown()
OverrideModulestoreFieldData.provider_classes = None
def make_one(self): def make_one(self):
return OverrideModulestoreFieldData.wrap(self.course, DictFieldData({ return OverrideModulestoreFieldData.wrap(self.course, DictFieldData({
'foo': 'bar', 'foo': 'bar',
......
...@@ -11,7 +11,6 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey ...@@ -11,7 +11,6 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator
from courseware.grades import ( from courseware.grades import (
field_data_cache_for_grading,
grade, grade,
iterate_grades_for, iterate_grades_for,
MaxScoresCache, MaxScoresCache,
...@@ -31,7 +30,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory ...@@ -31,7 +30,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
def _grade_with_errors(student, request, course, keep_raw_scores=False): def _grade_with_errors(student, course, keep_raw_scores=False):
"""This fake grade method will throw exceptions for student3 and """This fake grade method will throw exceptions for student3 and
student4, but allow any other students to go through normal grading. student4, but allow any other students to go through normal grading.
...@@ -42,7 +41,7 @@ def _grade_with_errors(student, request, course, keep_raw_scores=False): ...@@ -42,7 +41,7 @@ def _grade_with_errors(student, request, course, keep_raw_scores=False):
if student.username in ['student3', 'student4']: if student.username in ['student3', 'student4']:
raise Exception("I don't like {}".format(student.username)) raise Exception("I don't like {}".format(student.username))
return grade(student, request, course, keep_raw_scores=keep_raw_scores) return grade(student, course, keep_raw_scores=keep_raw_scores)
@attr('shard_1') @attr('shard_1')
...@@ -217,15 +216,6 @@ class TestFieldDataCacheScorableLocations(SharedModuleStoreTestCase): ...@@ -217,15 +216,6 @@ class TestFieldDataCacheScorableLocations(SharedModuleStoreTestCase):
CourseEnrollment.enroll(self.student, self.course.id) CourseEnrollment.enroll(self.student, self.course.id)
def test_field_data_cache_scorable_locations(self):
"""Only scorable locations should be in FieldDataCache.scorable_locations."""
fd_cache = field_data_cache_for_grading(self.course, self.student)
block_types = set(loc.block_type for loc in fd_cache.scorable_locations)
self.assertNotIn('video', block_types)
self.assertNotIn('html', block_types)
self.assertNotIn('discussion', block_types)
self.assertIn('problem', block_types)
class TestProgressSummary(TestCase): class TestProgressSummary(TestCase):
""" """
......
...@@ -339,16 +339,16 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -339,16 +339,16 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
) )
@override_settings(FIELD_OVERRIDE_PROVIDERS=( @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 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 = self.request_factory.get('')
request.user = self.mock_user request.user = self.mock_user
course = CourseFactory.create(enable_ccx=True) course = CourseFactory.create()
descriptor = ItemFactory(category='html', parent=course) descriptor = ItemFactory(category='html', parent=course)
field_data_cache = FieldDataCache( field_data_cache = FieldDataCache(
......
...@@ -256,13 +256,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl ...@@ -256,13 +256,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
- grade_breakdown : A breakdown of the major components that - grade_breakdown : A breakdown of the major components that
make up the final grade. (For display) make up the final grade. (For display)
""" """
return grades.grade(self.student_user, self.course)
fake_request = self.factory.get(
reverse('progress', kwargs={'course_id': self.course.id.to_deprecated_string()})
)
fake_request.user = self.student_user
return grades.grade(self.student_user, fake_request, self.course)
def get_progress_summary(self): def get_progress_summary(self):
""" """
...@@ -275,15 +269,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl ...@@ -275,15 +269,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
ungraded problems, and is good for displaying a course summary with due dates, ungraded problems, and is good for displaying a course summary with due dates,
etc. etc.
""" """
return grades.progress_summary(self.student_user, self.course)
fake_request = self.factory.get(
reverse('progress', kwargs={'course_id': self.course.id.to_deprecated_string()})
)
progress_summary = grades.progress_summary(
self.student_user, fake_request, self.course
)
return progress_summary
def check_grade_percent(self, percent): def check_grade_percent(self, percent):
""" """
......
...@@ -482,23 +482,7 @@ class TestGetHtmlMethod(BaseTestXmodule): ...@@ -482,23 +482,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
# it'll just fall back to the values in the VideoDescriptor. # it'll just fall back to the values in the VideoDescriptor.
self.assertIn("example_source.mp4", self.item_descriptor.render(STUDENT_VIEW).content) self.assertIn("example_source.mp4", self.item_descriptor.render(STUDENT_VIEW).content)
@patch('edxval.api.get_video_info') def test_get_html_with_mocked_edx_video_id(self):
def test_get_html_with_mocked_edx_video_id(self, mock_get_video_info):
mock_get_video_info.return_value = {
'url': '/edxval/video/example',
'edx_video_id': u'example',
'duration': 111.0,
'client_video_id': u'The example video',
'encoded_videos': [
{
'url': u'http://www.meowmix.com',
'file_size': 25556,
'bitrate': 9600,
'profile': u'desktop_mp4'
}
]
}
SOURCE_XML = """ SOURCE_XML = """
<video show_captions="true" <video show_captions="true"
display_name="A Name" display_name="A Name"
...@@ -558,7 +542,23 @@ class TestGetHtmlMethod(BaseTestXmodule): ...@@ -558,7 +542,23 @@ class TestGetHtmlMethod(BaseTestXmodule):
edx_video_id=data['edx_video_id'] edx_video_id=data['edx_video_id']
) )
self.initialize_module(data=DATA) self.initialize_module(data=DATA)
context = self.item_descriptor.render(STUDENT_VIEW).content
with patch('edxval.api.get_video_info') as mock_get_video_info:
mock_get_video_info.return_value = {
'url': '/edxval/video/example',
'edx_video_id': u'example',
'duration': 111.0,
'client_video_id': u'The example video',
'encoded_videos': [
{
'url': u'http://www.meowmix.com',
'file_size': 25556,
'bitrate': 9600,
'profile': u'desktop_mp4'
}
]
}
context = self.item_descriptor.render(STUDENT_VIEW).content
expected_context = dict(initial_context) expected_context = dict(initial_context)
expected_context['metadata'].update({ expected_context['metadata'].update({
......
...@@ -1340,7 +1340,7 @@ class ProgressPageTests(ModuleStoreTestCase): ...@@ -1340,7 +1340,7 @@ class ProgressPageTests(ModuleStoreTestCase):
self.assertContains(resp, u"Download Your Certificate") self.assertContains(resp, u"Download Your Certificate")
@ddt.data( @ddt.data(
*itertools.product(((55, 4, True), (55, 4, False)), (True, False)) *itertools.product(((46, 4, True), (46, 4, False)), (True, False))
) )
@ddt.unpack @ddt.unpack
def test_query_counts(self, (sql_calls, mongo_calls, self_paced), self_paced_enabled): def test_query_counts(self, (sql_calls, mongo_calls, self_paced), self_paced_enabled):
......
...@@ -8,6 +8,7 @@ import ddt ...@@ -8,6 +8,7 @@ import ddt
from mock import patch from mock import patch
from urllib import urlencode from urllib import urlencode
from lms.djangoapps.courseware.field_overrides import OverrideModulestoreFieldData
from lms.djangoapps.courseware.url_helpers import get_redirect_url from lms.djangoapps.courseware.url_helpers import get_redirect_url
from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
...@@ -197,3 +198,16 @@ class RenderXBlockTestMixin(object): ...@@ -197,3 +198,16 @@ class RenderXBlockTestMixin(object):
self.setup_course() self.setup_course()
self.setup_user(admin=False, enroll=True, login=True) self.setup_user(admin=False, enroll=True, login=True)
self.verify_response(url_params={'view': 'author_view'}, expected_response_code=400) 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
...@@ -38,6 +38,7 @@ from instructor.views.api import require_global_staff ...@@ -38,6 +38,7 @@ from instructor.views.api import require_global_staff
import shoppingcart import shoppingcart
import survey.utils import survey.utils
import survey.views import survey.views
from lms.djangoapps.ccx.utils import prep_course_for_grading
from certificates import api as certs_api from certificates import api as certs_api
from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.core.djangoapps.models.course_details import CourseDetails
from commerce.utils import EcommerceService from commerce.utils import EcommerceService
...@@ -681,6 +682,7 @@ def _progress(request, course_key, student_id): ...@@ -681,6 +682,7 @@ def _progress(request, course_key, student_id):
raise Http404 raise Http404
course = get_course_with_access(request.user, 'load', course_key, depth=None, check_if_enrolled=True) course = get_course_with_access(request.user, 'load', course_key, depth=None, check_if_enrolled=True)
prep_course_for_grading(course, request)
# check to see if there is a required survey that must be taken before # check to see if there is a required survey that must be taken before
# the user can access the course. # the user can access the course.
...@@ -714,16 +716,8 @@ def _progress(request, course_key, student_id): ...@@ -714,16 +716,8 @@ def _progress(request, course_key, student_id):
# additional DB lookup (this kills the Progress page in particular). # additional DB lookup (this kills the Progress page in particular).
student = User.objects.prefetch_related("groups").get(id=student.id) student = User.objects.prefetch_related("groups").get(id=student.id)
with outer_atomic(): courseware_summary = grades.progress_summary(student, course)
field_data_cache = grades.field_data_cache_for_grading(course, student) grade_summary = grades.grade(student, course)
scores_client = ScoresClient.from_field_data_cache(field_data_cache)
courseware_summary = grades.progress_summary(
student, request, course, field_data_cache=field_data_cache, scores_client=scores_client
)
grade_summary = grades.grade(
student, request, course, field_data_cache=field_data_cache, scores_client=scores_client
)
studio_url = get_studio_url(course, 'settings/grading') studio_url = get_studio_url(course, 'settings/grading')
if courseware_summary is None: if courseware_summary is None:
...@@ -1056,7 +1050,7 @@ def is_course_passed(course, grade_summary=None, student=None, request=None): ...@@ -1056,7 +1050,7 @@ def is_course_passed(course, grade_summary=None, student=None, request=None):
success_cutoff = min(nonzero_cutoffs) if nonzero_cutoffs else None success_cutoff = min(nonzero_cutoffs) if nonzero_cutoffs else None
if grade_summary is None: if grade_summary is None:
grade_summary = grades.grade(student, request, course) grade_summary = grades.grade(student, course)
return success_cutoff and grade_summary['percent'] >= success_cutoff return success_cutoff and grade_summary['percent'] >= success_cutoff
......
...@@ -342,11 +342,11 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase): ...@@ -342,11 +342,11 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
@ddt.data( @ddt.data(
# old mongo with cache # old mongo with cache
(ModuleStoreEnum.Type.mongo, 1, 6, 4, 18, 10), (ModuleStoreEnum.Type.mongo, 1, 6, 4, 17, 8),
(ModuleStoreEnum.Type.mongo, 50, 6, 4, 18, 10), (ModuleStoreEnum.Type.mongo, 50, 6, 4, 17, 8),
# split mongo: 3 queries, regardless of thread response size. # split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, 1, 3, 3, 18, 10), (ModuleStoreEnum.Type.split, 1, 3, 3, 17, 8),
(ModuleStoreEnum.Type.split, 50, 3, 3, 18, 10), (ModuleStoreEnum.Type.split, 50, 3, 3, 17, 8),
) )
@ddt.unpack @ddt.unpack
def test_number_of_mongo_queries( def test_number_of_mongo_queries(
......
...@@ -63,7 +63,7 @@ Graded sections: ...@@ -63,7 +63,7 @@ Graded sections:
Listing grading context for course {} Listing grading context for course {}
graded sections: graded sections:
[] []
all descriptors: all graded blocks:
length=0""".format(world.course_key) length=0""".format(world.course_key)
assert_in(expected_config, world.css_text('#data-grade-config-text')) assert_in(expected_config, world.css_text('#data-grade-config-text'))
......
...@@ -50,7 +50,7 @@ def offline_grade_calculation(course_key): ...@@ -50,7 +50,7 @@ def offline_grade_calculation(course_key):
request.user = student request.user = student
request.session = {} request.session = {}
gradeset = grades.grade(student, request, course, keep_raw_scores=True) gradeset = grades.grade(student, course, keep_raw_scores=True)
# Convert Score namedtuples to dicts: # Convert Score namedtuples to dicts:
totaled_scores = gradeset['totaled_scores'] totaled_scores = gradeset['totaled_scores']
for section in totaled_scores: for section in totaled_scores:
...@@ -89,7 +89,7 @@ def student_grades(student, request, course, keep_raw_scores=False, use_offline= ...@@ -89,7 +89,7 @@ def student_grades(student, request, course, keep_raw_scores=False, use_offline=
as use_offline. If use_offline is True then this will look for an offline computed gradeset in the DB. as use_offline. If use_offline is True then this will look for an offline computed gradeset in the DB.
''' '''
if not use_offline: if not use_offline:
return grades.grade(student, request, course, keep_raw_scores=keep_raw_scores) return grades.grade(student, course, keep_raw_scores=keep_raw_scores)
try: try:
ocg = models.OfflineComputedGrade.objects.get(user=student, course_id=course.id) ocg = models.OfflineComputedGrade.objects.get(user=student, course_id=course.id)
......
...@@ -16,7 +16,7 @@ from xmodule.modulestore.tests.factories import CourseFactory ...@@ -16,7 +16,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
from ..offline_gradecalc import offline_grade_calculation, student_grades from ..offline_gradecalc import offline_grade_calculation, student_grades
def mock_grade(_student, _request, course, **_kwargs): def mock_grade(_student, course, **_kwargs):
""" Return some fake grade data to mock grades.grade() """ """ Return some fake grade data to mock grades.grade() """
return { return {
'grade': u'Pass', 'grade': u'Pass',
...@@ -104,4 +104,4 @@ class TestOfflineGradeCalc(ModuleStoreTestCase): ...@@ -104,4 +104,4 @@ class TestOfflineGradeCalc(ModuleStoreTestCase):
offline_grade_calculation(self.course.id) offline_grade_calculation(self.course.id)
with patch('courseware.grades.grade', side_effect=AssertionError('Should not re-grade')): with patch('courseware.grades.grade', side_effect=AssertionError('Should not re-grade')):
result = student_grades(self.user, None, self.course, use_offline=True) result = student_grades(self.user, None, self.course, use_offline=True)
self.assertEqual(result, mock_grade(self.user, None, self.course)) self.assertEqual(result, mock_grade(self.user, self.course))
...@@ -392,6 +392,6 @@ class TestInstructorDashboardPerformance(ModuleStoreTestCase, LoginEnrollmentTes ...@@ -392,6 +392,6 @@ class TestInstructorDashboardPerformance(ModuleStoreTestCase, LoginEnrollmentTes
# check MongoDB calls count # check MongoDB calls count
url = reverse('spoc_gradebook', kwargs={'course_id': self.course.id}) url = reverse('spoc_gradebook', kwargs={'course_id': self.course.id})
with check_mongo_calls(8): with check_mongo_calls(7):
response = self.client.get(url) response = self.client.get(url)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
...@@ -24,6 +24,7 @@ from courseware.models import StudentModule ...@@ -24,6 +24,7 @@ from courseware.models import StudentModule
from certificates.models import GeneratedCertificate from certificates.models import GeneratedCertificate
from django.db.models import Count from django.db.models import Count
from certificates.models import CertificateStatuses from certificates.models import CertificateStatuses
from courseware.grades import grading_context_for_course
STUDENT_FEATURES = ('id', 'username', 'first_name', 'last_name', 'is_staff', 'email') STUDENT_FEATURES = ('id', 'username', 'first_name', 'last_name', 'is_staff', 'email')
...@@ -490,14 +491,14 @@ def dump_grading_context(course): ...@@ -490,14 +491,14 @@ def dump_grading_context(course):
msg += hbar msg += hbar
msg += "Listing grading context for course %s\n" % course.id.to_deprecated_string() msg += "Listing grading context for course %s\n" % course.id.to_deprecated_string()
gcontext = course.grading_context gcontext = grading_context_for_course(course)
msg += "graded sections:\n" msg += "graded sections:\n"
msg += '%s\n' % gcontext['graded_sections'].keys() msg += '%s\n' % gcontext['all_graded_sections'].keys()
for (gsomething, gsvals) in gcontext['graded_sections'].items(): for (gsomething, gsvals) in gcontext['all_graded_sections'].items():
msg += "--> Section %s:\n" % (gsomething) msg += "--> Section %s:\n" % (gsomething)
for sec in gsvals: for sec in gsvals:
sdesc = sec['section_descriptor'] sdesc = sec['section_block']
frmat = getattr(sdesc, 'format', None) frmat = getattr(sdesc, 'format', None)
aname = '' aname = ''
if frmat in graders: if frmat in graders:
...@@ -512,7 +513,7 @@ def dump_grading_context(course): ...@@ -512,7 +513,7 @@ def dump_grading_context(course):
notes = ', score by attempt!' notes = ', score by attempt!'
msg += " %s (format=%s, Assignment=%s%s)\n"\ msg += " %s (format=%s, Assignment=%s%s)\n"\
% (sdesc.display_name, frmat, aname, notes) % (sdesc.display_name, frmat, aname, notes)
msg += "all descriptors:\n" msg += "all graded blocks:\n"
msg += "length=%d\n" % len(gcontext['all_descriptors']) msg += "length=%d\n" % len(gcontext['all_graded_blocks'])
msg = '<pre>%s</pre>' % msg.replace('<', '&lt;') msg = '<pre>%s</pre>' % msg.replace('<', '&lt;')
return msg return msg
...@@ -285,7 +285,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): ...@@ -285,7 +285,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
user_b.username, user_b.username,
course.id, course.id,
cohort_name_header, cohort_name_header,
'' u'Default Group',
) )
@patch('instructor_task.tasks_helper._get_current_task') @patch('instructor_task.tasks_helper._get_current_task')
...@@ -685,7 +685,7 @@ class TestProblemReportSplitTestContent(TestReportMixin, TestConditionalContent, ...@@ -685,7 +685,7 @@ class TestProblemReportSplitTestContent(TestReportMixin, TestConditionalContent,
def test_problem_grade_report(self): def test_problem_grade_report(self):
""" """
Test that we generate the correct the correct grade report when dealing with A/B tests. Test that we generate the correct grade report when dealing with A/B tests.
In order to verify that the behavior of the grade report is correct, we submit answers for problems In order to verify that the behavior of the grade report is correct, we submit answers for problems
that the student won't have access to. A/B tests won't restrict access to the problems, but it should that the student won't have access to. A/B tests won't restrict access to the problems, but it should
......
...@@ -731,7 +731,7 @@ COURSE_CATALOG_API_URL = ENV_TOKENS.get('COURSE_CATALOG_API_URL', COURSE_CATALOG ...@@ -731,7 +731,7 @@ COURSE_CATALOG_API_URL = ENV_TOKENS.get('COURSE_CATALOG_API_URL', COURSE_CATALOG
##### Custom Courses for EdX ##### ##### Custom Courses for EdX #####
if FEATURES.get('CUSTOM_COURSES_EDX'): if FEATURES.get('CUSTOM_COURSES_EDX'):
INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon') INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon')
FIELD_OVERRIDE_PROVIDERS += ( MODULESTORE_FIELD_OVERRIDE_PROVIDERS += (
'lms.djangoapps.ccx.overrides.CustomCoursesForEdxOverrideProvider', 'lms.djangoapps.ccx.overrides.CustomCoursesForEdxOverrideProvider',
) )
CCX_MAX_STUDENTS_ALLOWED = ENV_TOKENS.get('CCX_MAX_STUDENTS_ALLOWED', CCX_MAX_STUDENTS_ALLOWED) CCX_MAX_STUDENTS_ALLOWED = ENV_TOKENS.get('CCX_MAX_STUDENTS_ALLOWED', CCX_MAX_STUDENTS_ALLOWED)
......
...@@ -577,11 +577,6 @@ JWT_AUTH.update({ ...@@ -577,11 +577,6 @@ JWT_AUTH.update({
'JWT_AUDIENCE': 'test-key', 'JWT_AUDIENCE': 'test-key',
}) })
# Disable the use of the plugin manager in the transformer registry for
# better performant unit tests.
from openedx.core.lib.block_structure.transformer_registry import TransformerRegistry
TransformerRegistry.USE_PLUGIN_MANAGER = False
# Set the default Oauth2 Provider Model so that migrations can run in # Set the default Oauth2 Provider Model so that migrations can run in
# verbose mode # verbose mode
OAUTH2_PROVIDER_APPLICATION_MODEL = 'oauth2_provider.Application' OAUTH2_PROVIDER_APPLICATION_MODEL = 'oauth2_provider.Application'
......
...@@ -299,7 +299,7 @@ GRADES_DOWNLOAD_ROUTING_KEY = HIGH_MEM_QUEUE ...@@ -299,7 +299,7 @@ GRADES_DOWNLOAD_ROUTING_KEY = HIGH_MEM_QUEUE
##### Custom Courses for EdX ##### ##### Custom Courses for EdX #####
if FEATURES.get('CUSTOM_COURSES_EDX'): if FEATURES.get('CUSTOM_COURSES_EDX'):
INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon') INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon')
FIELD_OVERRIDE_PROVIDERS += ( MODULESTORE_FIELD_OVERRIDE_PROVIDERS += (
'lms.djangoapps.ccx.overrides.CustomCoursesForEdxOverrideProvider', 'lms.djangoapps.ccx.overrides.CustomCoursesForEdxOverrideProvider',
) )
......
...@@ -3,28 +3,9 @@ Helpers for Course Blocks tests. ...@@ -3,28 +3,9 @@ Helpers for Course Blocks tests.
""" """
from openedx.core.lib.block_structure.cache import BlockStructureCache from openedx.core.lib.block_structure.cache import BlockStructureCache
from openedx.core.lib.block_structure.transformer_registry import TransformerRegistry
from ..api import get_cache from ..api import get_cache
class EnableTransformerRegistryMixin(object):
"""
Mixin that enables the TransformerRegistry to USE_PLUGIN_MANAGER for
finding registered transformers. USE_PLUGIN_MANAGER is set to False
for LMS unit tests to speed up performance of the unit tests, so all
registered transformers in the platform do not need to be collected.
This Mixin is expected to be used by Tests for integration testing
with all registered transformers.
"""
def setUp(self, **kwargs):
super(EnableTransformerRegistryMixin, self).setUp(**kwargs)
TransformerRegistry.USE_PLUGIN_MANAGER = True
def tearDown(self):
super(EnableTransformerRegistryMixin, self).tearDown()
TransformerRegistry.USE_PLUGIN_MANAGER = False
def is_course_in_block_structure_cache(course_key, store): def is_course_in_block_structure_cache(course_key, store):
""" """
Returns whether the given course is in the Block Structure cache. Returns whether the given course is in the Block Structure cache.
......
...@@ -7,15 +7,13 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase ...@@ -7,15 +7,13 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from ..api import get_block_structure_manager from ..api import get_block_structure_manager
from .helpers import is_course_in_block_structure_cache, EnableTransformerRegistryMixin from .helpers import is_course_in_block_structure_cache
class CourseBlocksSignalTest(EnableTransformerRegistryMixin, ModuleStoreTestCase): class CourseBlocksSignalTest(ModuleStoreTestCase):
""" """
Tests for the Course Blocks signal Tests for the Course Blocks signal
""" """
ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
def setUp(self): def setUp(self):
super(CourseBlocksSignalTest, self).setUp() super(CourseBlocksSignalTest, self).setUp()
self.course = CourseFactory.create() self.course = CourseFactory.create()
......
...@@ -20,7 +20,7 @@ from lms.djangoapps import django_comment_client ...@@ -20,7 +20,7 @@ from lms.djangoapps import django_comment_client
from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.core.djangoapps.models.course_details import CourseDetails
from static_replace.models import AssetBaseUrlConfig from static_replace.models import AssetBaseUrlConfig
from util.date_utils import strftime_localized from util.date_utils import strftime_localized
from xmodule import course_metadata_utils from xmodule import course_metadata_utils, block_metadata_utils
from xmodule.course_module import CourseDescriptor, DEFAULT_START_DATE from xmodule.course_module import CourseDescriptor, DEFAULT_START_DATE
from xmodule.error_module import ErrorDescriptor from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
...@@ -317,14 +317,14 @@ class CourseOverview(TimeStampedModel): ...@@ -317,14 +317,14 @@ class CourseOverview(TimeStampedModel):
""" """
Returns this course's URL name. Returns this course's URL name.
""" """
return course_metadata_utils.url_name_for_course_location(self.location) return block_metadata_utils.url_name_for_block(self)
@property @property
def display_name_with_default(self): def display_name_with_default(self):
""" """
Return reasonable display name for the course. Return reasonable display name for the course.
""" """
return course_metadata_utils.display_name_with_default(self) return block_metadata_utils.display_name_with_default(self)
@property @property
def display_name_with_default_escaped(self): def display_name_with_default_escaped(self):
...@@ -338,7 +338,7 @@ class CourseOverview(TimeStampedModel): ...@@ -338,7 +338,7 @@ class CourseOverview(TimeStampedModel):
migrate and test switching to display_name_with_default, which is no migrate and test switching to display_name_with_default, which is no
longer escaped. longer escaped.
""" """
return course_metadata_utils.display_name_with_default_escaped(self) return block_metadata_utils.display_name_with_default_escaped(self)
def has_started(self): def has_started(self):
""" """
......
...@@ -72,6 +72,9 @@ class BlockStructure(object): ...@@ -72,6 +72,9 @@ class BlockStructure(object):
""" """
return self.get_block_keys() return self.get_block_keys()
def __len__(self):
return len(self._block_relations)
#--- Block structure relation methods ---# #--- Block structure relation methods ---#
def get_parents(self, usage_key): def get_parents(self, usage_key):
...@@ -149,6 +152,7 @@ class BlockStructure(object): ...@@ -149,6 +152,7 @@ class BlockStructure(object):
self, self,
filter_func=None, filter_func=None,
yield_descendants_of_unyielded=False, yield_descendants_of_unyielded=False,
start_node=None,
): ):
""" """
Performs a topological sort of the block structure and yields Performs a topological sort of the block structure and yields
...@@ -163,7 +167,7 @@ class BlockStructure(object): ...@@ -163,7 +167,7 @@ class BlockStructure(object):
traverse_topologically method. traverse_topologically method.
""" """
return traverse_topologically( return traverse_topologically(
start_node=self.root_block_usage_key, start_node=start_node or self.root_block_usage_key,
get_parents=self.get_parents, get_parents=self.get_parents,
get_children=self.get_children, get_children=self.get_children,
filter_func=filter_func, filter_func=filter_func,
...@@ -173,6 +177,7 @@ class BlockStructure(object): ...@@ -173,6 +177,7 @@ class BlockStructure(object):
def post_order_traversal( def post_order_traversal(
self, self,
filter_func=None, filter_func=None,
start_node=None,
): ):
""" """
Performs a post-order sort of the block structure and yields Performs a post-order sort of the block structure and yields
...@@ -187,7 +192,7 @@ class BlockStructure(object): ...@@ -187,7 +192,7 @@ class BlockStructure(object):
traverse_post_order method. traverse_post_order method.
""" """
return traverse_post_order( return traverse_post_order(
start_node=self.root_block_usage_key, start_node=start_node or self.root_block_usage_key,
get_children=self.get_children, get_children=self.get_children,
filter_func=filter_func, filter_func=filter_func,
) )
...@@ -267,19 +272,119 @@ class BlockStructure(object): ...@@ -267,19 +272,119 @@ class BlockStructure(object):
block_relations[usage_key] = _BlockRelations() block_relations[usage_key] = _BlockRelations()
class _BlockData(object): class FieldData(object):
""" """
Data structure to encapsulate collected data for a single block. Data structure to encapsulate collected fields.
""" """
def class_field_names(self):
"""
Returns list of names of fields that are defined directly
on the class. Can be overridden by subclasses. All other
fields are assumed to be stored in the self.fields dict.
"""
return ['fields']
def __init__(self): def __init__(self):
# Map of xblock field name to the field's value for this block. # Map of field name to the field's value for this block.
# dict {string: any picklable type} # dict {string: any picklable type}
self.xblock_fields = {} self.fields = {}
def __getattr__(self, field_name):
if self._is_own_field(field_name):
return super(FieldData, self).__getattr__(field_name)
try:
return self.fields[field_name]
except KeyError:
raise AttributeError("Field {0} does not exist".format(field_name))
def __setattr__(self, field_name, field_value):
if self._is_own_field(field_name):
return super(FieldData, self).__setattr__(field_name, field_value)
else:
self.fields[field_name] = field_value
def __delattr__(self, field_name):
if self._is_own_field(field_name):
return super(FieldData, self).__delattr__(field_name)
else:
delattr(self.fields, field_name)
def _is_own_field(self, field_name):
"""
Returns whether the given field_name is the name of an
actual field of this class.
"""
return field_name in self.class_field_names()
# Map of transformer name to the transformer's data for this
# block. class TransformerData(FieldData):
# defaultdict {string: dict} """
self.transformer_data = defaultdict(dict) Data structure to encapsulate collected data for a transformer.
"""
pass
class TransformerDataMap(dict):
"""
A map of Transformer name to its corresponding TransformerData.
The map can be accessed by the Transformer's name or the
Transformer's class type.
"""
def __getitem__(self, key):
key = self._translate_key(key)
return dict.__getitem__(self, key)
def __setitem__(self, key, value):
key = self._translate_key(key)
dict.__setitem__(self, key, value)
def __delitem__(self, key):
key = self._translate_key(key)
dict.__delitem__(self, key)
def get_or_create(self, key):
"""
Returns the TransformerData associated with the given
key. If not found, creates and returns a new TransformerData
and maps it to the given key.
"""
try:
return self[key]
except KeyError:
new_transformer_data = TransformerData()
self[key] = new_transformer_data
return new_transformer_data
def _translate_key(self, key):
"""
Allows the given key to be either the transformer's class or name,
always returning the transformer's name. This allows
TransformerDataMap to be accessed in either of the following ways:
map[TransformerClass] or
map['transformer_name']
"""
try:
return key.name()
except AttributeError:
return key
class BlockData(FieldData):
"""
Data structure to encapsulate collected data for a single block.
"""
def class_field_names(self):
return super(BlockData, self).class_field_names() + ['location', 'transformer_data']
def __init__(self, usage_key):
super(BlockData, self).__init__()
# Location (or usage key) of the block.
self.location = usage_key
# Map of transformer name to its block-specific data.
self.transformer_data = TransformerDataMap()
class BlockStructureBlockData(BlockStructure): class BlockStructureBlockData(BlockStructure):
...@@ -292,12 +397,31 @@ class BlockStructureBlockData(BlockStructure): ...@@ -292,12 +397,31 @@ class BlockStructureBlockData(BlockStructure):
# Map of a block's usage key to its collected data, including # Map of a block's usage key to its collected data, including
# its xBlock fields and block-specific transformer data. # its xBlock fields and block-specific transformer data.
# defaultdict {UsageKey: _BlockData} # dict {UsageKey: BlockData}
self._block_data_map = defaultdict(_BlockData) self._block_data_map = {}
# Map of a transformer's name to its non-block-specific data. # Map of a transformer's name to its non-block-specific data.
# defaultdict {string: dict} self.transformer_data = TransformerDataMap()
self._transformer_data = defaultdict(dict)
def iteritems(self):
"""
Returns iterator of (UsageKey, BlockData) pairs for all
blocks in the BlockStructure.
"""
return self._block_data_map.iteritems()
def itervalues(self):
"""
Returns iterator of BlockData for all blocks in the
BlockStructure.
"""
return self._block_data_map.itervalues()
def __getitem__(self, usage_key):
"""
Returns the BlockData associated with the given key.
"""
return self._block_data_map.get(usage_key)
def get_xblock_field(self, usage_key, field_name, default=None): def get_xblock_field(self, usage_key, field_name, default=None):
""" """
...@@ -316,7 +440,7 @@ class BlockStructureBlockData(BlockStructure): ...@@ -316,7 +440,7 @@ class BlockStructureBlockData(BlockStructure):
not found. not found.
""" """
block_data = self._block_data_map.get(usage_key) block_data = self._block_data_map.get(usage_key)
return block_data.xblock_fields.get(field_name, default) if block_data else default return getattr(block_data, field_name, default) if block_data else default
def get_transformer_data(self, transformer, key, default=None): def get_transformer_data(self, transformer, key, default=None):
""" """
...@@ -330,7 +454,10 @@ class BlockStructureBlockData(BlockStructure): ...@@ -330,7 +454,10 @@ class BlockStructureBlockData(BlockStructure):
key (string) - A dictionary key to the transformer's data key (string) - A dictionary key to the transformer's data
that is requested. that is requested.
""" """
return self._transformer_data.get(transformer.name(), {}).get(key, default) try:
return getattr(self.transformer_data[transformer], key, default)
except KeyError:
return default
def set_transformer_data(self, transformer, key, value): def set_transformer_data(self, transformer, key, value):
""" """
...@@ -346,7 +473,23 @@ class BlockStructureBlockData(BlockStructure): ...@@ -346,7 +473,23 @@ class BlockStructureBlockData(BlockStructure):
value (any picklable type) - The value to associate with the value (any picklable type) - The value to associate with the
given key for the given transformer's data. given key for the given transformer's data.
""" """
self._transformer_data[transformer.name()][key] = value setattr(self.transformer_data.get_or_create(transformer), key, value)
def get_transformer_block_data(self, usage_key, transformer):
"""
Returns the TransformerData for the given
transformer for the block identified by the given usage_key.
Raises KeyError if not found.
Arguments:
usage_key (UsageKey) - Usage key of the block whose
transformer data is requested.
transformer (BlockStructureTransformer) - The transformer
whose dictionary data is requested.
"""
return self._block_data_map[usage_key].transformer_data[transformer]
def get_transformer_block_field(self, usage_key, transformer, key, default=None): def get_transformer_block_field(self, usage_key, transformer, key, default=None):
""" """
...@@ -367,8 +510,11 @@ class BlockStructureBlockData(BlockStructure): ...@@ -367,8 +510,11 @@ class BlockStructureBlockData(BlockStructure):
default (any type) - The value to return if a dictionary default (any type) - The value to return if a dictionary
entry is not found. entry is not found.
""" """
transformer_data = self.get_transformer_block_data(usage_key, transformer) try:
return transformer_data.get(key, default) transformer_data = self.get_transformer_block_data(usage_key, transformer)
except KeyError:
return default
return getattr(transformer_data, key, default)
def set_transformer_block_field(self, usage_key, transformer, key, value): def set_transformer_block_field(self, usage_key, transformer, key, value):
""" """
...@@ -388,30 +534,11 @@ class BlockStructureBlockData(BlockStructure): ...@@ -388,30 +534,11 @@ class BlockStructureBlockData(BlockStructure):
given key for the given transformer's data for the given key for the given transformer's data for the
requested block. requested block.
""" """
self._block_data_map[usage_key].transformer_data[transformer.name()][key] = value setattr(
self._get_or_create_block(usage_key).transformer_data.get_or_create(transformer),
def get_transformer_block_data(self, usage_key, transformer): key,
""" value,
Returns the entire transformer data dict for the given )
transformer for the block identified by the given usage_key;
returns an empty dict {} if not found.
Arguments:
usage_key (UsageKey) - Usage key of the block whose
transformer data is requested.
transformer (BlockStructureTransformer) - The transformer
whose dictionary data is requested.
key (string) - A dictionary key to the transformer's data
that is requested.
"""
default = {}
block_data = self._block_data_map.get(usage_key)
if not block_data:
return default
else:
return block_data.transformer_data.get(transformer.name(), default)
def remove_transformer_block_field(self, usage_key, transformer, key): def remove_transformer_block_field(self, usage_key, transformer, key):
""" """
...@@ -425,8 +552,11 @@ class BlockStructureBlockData(BlockStructure): ...@@ -425,8 +552,11 @@ class BlockStructureBlockData(BlockStructure):
transformer (BlockStructureTransformer) - The transformer transformer (BlockStructureTransformer) - The transformer
whose data entry is to be deleted. whose data entry is to be deleted.
""" """
transformer_block_data = self.get_transformer_block_data(usage_key, transformer) try:
transformer_block_data.pop(key, None) transformer_block_data = self.get_transformer_block_data(usage_key, transformer)
delattr(transformer_block_data, key)
except (AttributeError, KeyError):
pass
def remove_block(self, usage_key, keep_descendants): def remove_block(self, usage_key, keep_descendants):
""" """
...@@ -527,6 +657,19 @@ class BlockStructureBlockData(BlockStructure): ...@@ -527,6 +657,19 @@ class BlockStructureBlockData(BlockStructure):
raise TransformerException('VERSION attribute is not set on transformer {0}.', transformer.name()) raise TransformerException('VERSION attribute is not set on transformer {0}.', transformer.name())
self.set_transformer_data(transformer, TRANSFORMER_VERSION_KEY, transformer.VERSION) self.set_transformer_data(transformer, TRANSFORMER_VERSION_KEY, transformer.VERSION)
def _get_or_create_block(self, usage_key):
"""
Returns the BlockData associated with the given usage_key.
If not found, creates and returns a new BlockData and
maps it to the given key.
"""
try:
return self._block_data_map[usage_key]
except KeyError:
block_data = BlockData(usage_key)
self._block_data_map[usage_key] = block_data
return block_data
class BlockStructureModulestoreData(BlockStructureBlockData): class BlockStructureModulestoreData(BlockStructureBlockData):
""" """
...@@ -599,23 +742,19 @@ class BlockStructureModulestoreData(BlockStructureBlockData): ...@@ -599,23 +742,19 @@ class BlockStructureModulestoreData(BlockStructureBlockData):
Iterates through all instantiated xBlocks that were added and Iterates through all instantiated xBlocks that were added and
collects all xBlock fields that were requested. collects all xBlock fields that were requested.
""" """
if not self._requested_xblock_fields:
return
for xblock_usage_key, xblock in self._xblock_map.iteritems(): for xblock_usage_key, xblock in self._xblock_map.iteritems():
block_data = self._get_or_create_block(xblock_usage_key)
for field_name in self._requested_xblock_fields: for field_name in self._requested_xblock_fields:
self._set_xblock_field(xblock_usage_key, xblock, field_name) self._set_xblock_field(block_data, xblock, field_name)
def _set_xblock_field(self, usage_key, xblock, field_name): def _set_xblock_field(self, block_data, xblock, field_name):
""" """
Updates the given block's xBlock fields data with the xBlock Updates the given block's xBlock fields data with the xBlock
value for the given field name. value for the given field name.
Arguments: Arguments:
usage_key (UsageKey) - Usage key of the given xBlock. This block_data (BlockData) - A BlockStructure BlockData
value is passed in separately as opposed to retrieving object.
it from the given xBlock since this interface is
agnostic to and decoupled from the xBlock interface.
xblock (XBlock) - An instantiated XBlock object whose xblock (XBlock) - An instantiated XBlock object whose
field is being accessed and collected for later field is being accessed and collected for later
...@@ -625,4 +764,4 @@ class BlockStructureModulestoreData(BlockStructureBlockData): ...@@ -625,4 +764,4 @@ class BlockStructureModulestoreData(BlockStructureBlockData):
being collected and stored. being collected and stored.
""" """
if hasattr(xblock, field_name): if hasattr(xblock, field_name):
self._block_data_map[usage_key].xblock_fields[field_name] = getattr(xblock, field_name) setattr(block_data, field_name, getattr(xblock, field_name))
...@@ -40,7 +40,7 @@ class BlockStructureCache(object): ...@@ -40,7 +40,7 @@ class BlockStructureCache(object):
""" """
data_to_cache = ( data_to_cache = (
block_structure._block_relations, block_structure._block_relations,
block_structure._transformer_data, block_structure.transformer_data,
block_structure._block_data_map, block_structure._block_data_map,
) )
zp_data_to_cache = zpickle(data_to_cache) zp_data_to_cache = zpickle(data_to_cache)
...@@ -99,7 +99,7 @@ class BlockStructureCache(object): ...@@ -99,7 +99,7 @@ class BlockStructureCache(object):
block_relations, transformer_data, block_data_map = zunpickle(zp_data_from_cache) block_relations, transformer_data, block_data_map = zunpickle(zp_data_from_cache)
block_structure = BlockStructureModulestoreData(root_block_usage_key) block_structure = BlockStructureModulestoreData(root_block_usage_key)
block_structure._block_relations = block_relations block_structure._block_relations = block_relations
block_structure._transformer_data = transformer_data block_structure.transformer_data = transformer_data
block_structure._block_data_map = block_data_map block_structure._block_data_map = block_data_map
return block_structure return block_structure
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
Top-level module for the Block Structure framework with a class for managing Top-level module for the Block Structure framework with a class for managing
BlockStructures. BlockStructures.
""" """
from contextlib import contextmanager
from .cache import BlockStructureCache from .cache import BlockStructureCache
from .factory import BlockStructureFactory from .factory import BlockStructureFactory
from .exceptions import UsageKeyNotInBlockStructure from .exceptions import UsageKeyNotInBlockStructure
...@@ -87,12 +89,13 @@ class BlockStructureManager(object): ...@@ -87,12 +89,13 @@ class BlockStructureManager(object):
) )
cache_miss = block_structure is None cache_miss = block_structure is None
if cache_miss or BlockStructureTransformers.is_collected_outdated(block_structure): if cache_miss or BlockStructureTransformers.is_collected_outdated(block_structure):
block_structure = BlockStructureFactory.create_from_modulestore( with self._bulk_operations():
self.root_block_usage_key, block_structure = BlockStructureFactory.create_from_modulestore(
self.modulestore self.root_block_usage_key,
) self.modulestore
BlockStructureTransformers.collect(block_structure) )
self.block_structure_cache.add(block_structure) BlockStructureTransformers.collect(block_structure)
self.block_structure_cache.add(block_structure)
return block_structure return block_structure
def update_collected(self): def update_collected(self):
...@@ -111,3 +114,15 @@ class BlockStructureManager(object): ...@@ -111,3 +114,15 @@ class BlockStructureManager(object):
root block key. root block key.
""" """
self.block_structure_cache.delete(self.root_block_usage_key) self.block_structure_cache.delete(self.root_block_usage_key)
@contextmanager
def _bulk_operations(self):
"""
A context manager for notifying the store of bulk operations.
"""
try:
course_key = self.root_block_usage_key.course_key
except AttributeError:
course_key = None
with self.modulestore.bulk_operations(course_key):
yield
...@@ -68,6 +68,13 @@ class MockModulestore(object): ...@@ -68,6 +68,13 @@ class MockModulestore(object):
raise ItemNotFoundError raise ItemNotFoundError
return item return item
@contextmanager
def bulk_operations(self, ignore): # pylint: disable=unused-argument
"""
A context manager for notifying the store of bulk operations.
"""
yield
class MockCache(object): class MockCache(object):
""" """
......
...@@ -138,17 +138,19 @@ class TestBlockStructureData(TestCase, ChildrenMapTestMixin): ...@@ -138,17 +138,19 @@ class TestBlockStructureData(TestCase, ChildrenMapTestMixin):
# verify fields have not been collected yet # verify fields have not been collected yet
for block in blocks: for block in blocks:
bs_block = block_structure[block.location]
for field in fields: for field in fields:
self.assertIsNone(block_structure.get_xblock_field(block.location, field)) self.assertIsNone(getattr(bs_block, field, None))
# collect fields # collect fields
block_structure._collect_requested_xblock_fields() block_structure._collect_requested_xblock_fields()
# verify values of collected fields # verify values of collected fields
for block in blocks: for block in blocks:
bs_block = block_structure[block.location]
for field in fields: for field in fields:
self.assertEquals( self.assertEquals(
block_structure.get_xblock_field(block.location, field), getattr(bs_block, field, None),
block.field_map.get(field), block.field_map.get(field),
) )
......
""" """
Utilities related to caching. Utilities related to caching.
""" """
import collections
import cPickle as pickle import cPickle as pickle
import functools import functools
import zlib import zlib
...@@ -40,6 +41,48 @@ def memoize_in_request_cache(request_cache_attr_name=None): ...@@ -40,6 +41,48 @@ def memoize_in_request_cache(request_cache_attr_name=None):
return _decorator return _decorator
class memoized(object): # pylint: disable=invalid-name
"""
Decorator. Caches a function's return value each time it is called.
If called later with the same arguments, the cached value is returned
(not reevaluated).
https://wiki.python.org/moin/PythonDecoratorLibrary#Memoize
WARNING: Only use this memoized decorator for caching data that
is constant throughout the lifetime of a gunicorn worker process,
is costly to compute, and is required often. Otherwise, it can lead to
unwanted memory leakage.
"""
def __init__(self, func):
self.func = func
self.cache = {}
def __call__(self, *args):
if not isinstance(args, collections.Hashable):
# uncacheable. a list, for instance.
# better to not cache than blow up.
return self.func(*args)
if args in self.cache:
return self.cache[args]
else:
value = self.func(*args)
self.cache[args] = value
return value
def __repr__(self):
"""
Return the function's docstring.
"""
return self.func.__doc__
def __get__(self, obj, objtype):
"""
Support instance methods.
"""
return functools.partial(self.__call__, obj)
def hashvalue(arg): def hashvalue(arg):
""" """
If arg is an xblock, use its location. otherwise just turn it into a string If arg is an xblock, use its location. otherwise just turn it into a string
......
...@@ -51,6 +51,7 @@ setup( ...@@ -51,6 +51,7 @@ setup(
"visibility = lms.djangoapps.course_blocks.transformers.visibility:VisibilityTransformer", "visibility = lms.djangoapps.course_blocks.transformers.visibility:VisibilityTransformer",
"course_blocks_api = lms.djangoapps.course_api.blocks.transformers.blocks_api:BlocksAPITransformer", "course_blocks_api = lms.djangoapps.course_api.blocks.transformers.blocks_api:BlocksAPITransformer",
"proctored_exam = lms.djangoapps.course_api.blocks.transformers.proctored_exam:ProctoredExamTransformer", "proctored_exam = lms.djangoapps.course_api.blocks.transformers.proctored_exam:ProctoredExamTransformer",
"grades = lms.djangoapps.courseware.transformers.grades:GradesTransformer",
], ],
} }
) )
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