diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 34253fb657..adea910ae7 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -54,6 +54,7 @@ from openedx.core.djangoapps.bookmarks.services import BookmarksService from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, unquote_slashes, quote_slashes from lms.djangoapps.verify_student.services import ReverificationService from openedx.core.djangoapps.credit.services import CreditService +from openedx.core.djangoapps.util.user_utils import SystemUser from openedx.core.lib.xblock_utils import ( replace_course_urls, replace_jump_to_id_urls, @@ -836,10 +837,10 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id # Not that the access check needs to happen after the descriptor is bound # for the student, since there may be field override data for the student # that affects xblock visibility. - if getattr(user, 'known', True): + user_needs_access_check = getattr(user, 'known', True) and not isinstance(user, SystemUser) + if user_needs_access_check: if not has_access(user, 'load', descriptor, course_id): return None - return descriptor diff --git a/lms/djangoapps/courseware/tests/test_self_paced_overrides.py b/lms/djangoapps/courseware/tests/test_self_paced_overrides.py index 14fa347c36..467459f35d 100644 --- a/lms/djangoapps/courseware/tests/test_self_paced_overrides.py +++ b/lms/djangoapps/courseware/tests/test_self_paced_overrides.py @@ -26,6 +26,7 @@ class SelfPacedDateOverrideTest(ModuleStoreTestCase): """ def setUp(self): + self.reset_setting_cache_variables() super(SelfPacedDateOverrideTest, self).setUp() SelfPacedConfiguration(enabled=True).save() @@ -35,8 +36,15 @@ class SelfPacedDateOverrideTest(ModuleStoreTestCase): self.future = self.now + datetime.timedelta(days=30) def tearDown(self): + self.reset_setting_cache_variables() super(SelfPacedDateOverrideTest, self).tearDown() + def reset_setting_cache_variables(self): + """ + The overridden settings for this class get cached on class variables. + Reset those to None before and after running the test to ensure clean + behavior. + """ OverrideFieldData.provider_classes = None OverrideModulestoreFieldData.provider_classes = None @@ -98,9 +106,9 @@ class SelfPacedDateOverrideTest(ModuleStoreTestCase): beta_tester = BetaTesterFactory(course_key=self_paced_course.id) # Verify course is `self_paced` and course has start date but not section. - self.assertTrue(self_paced_course.self_paced, "Course is self_paced") - self.assertEqual(self_paced_course.start, one_month_from_now, "Course has start date") - self.assertIsNone(self_paced_section.start, "Section start date is None") + self.assertTrue(self_paced_course.self_paced) + self.assertEqual(self_paced_course.start, one_month_from_now) + self.assertIsNone(self_paced_section.start) # Verify that non-staff user do not have access to the course self.assertFalse(has_access(self.non_staff_user, 'load', self_paced_course)) diff --git a/lms/djangoapps/courseware/tests/test_transformers.py b/lms/djangoapps/courseware/tests/test_transformers.py new file mode 100644 index 0000000000..afb5d96902 --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_transformers.py @@ -0,0 +1,250 @@ +""" +Test the behavior of the GradesTransformer +""" + +import datetime +import pytz +import random + +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import check_mongo_calls + +from lms.djangoapps.course_blocks.api import _get_cache +from lms.djangoapps.course_blocks.api import get_course_blocks +from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase +from ..transformers.grades import GradesTransformer + + +class GradesTransformerTestCase(CourseStructureTestCase): + """ + Verify behavior of the GradesTransformer + """ + + TRANSFORMER_CLASS_TO_TEST = GradesTransformer + + problem_metadata = { + u'graded': True, + u'weight': 1, + u'due': datetime.datetime(2099, 3, 15, 12, 30, 0, tzinfo=pytz.utc), + } + + def setUp(self): + super(GradesTransformerTestCase, self).setUp() + password = u'test' + self.student = UserFactory.create(is_staff=False, username=u'test_student', password=password) + self.client.login(username=self.student.username, password=password) + + def assert_collected_xblock_fields(self, block_structure, usage_key, **expectations): + """ + Given a block structure, a block usage key, and a list of keyword + arguments representing XBlock fields, verify that the block structure + has the specified values for each XBlock field. + """ + self.assertGreater(len(expectations), 0) + for field in expectations: + # Append our custom message to the default assertEqual error message + self.longMessage = True # pylint: disable=invalid-name + self.assertEqual( + expectations[field], + block_structure.get_xblock_field(usage_key, field), + msg=u'in field {},'.format(repr(field)), + ) + + def assert_collected_transformer_block_fields(self, block_structure, usage_key, transformer_class, **expectations): + """ + Given a block structure, a block usage key, a transformer, and a list + of keyword arguments representing transformer block fields, verify that + the block structure has the specified values for each transformer block + field. + """ + self.assertGreater(len(expectations), 0) + # Append our custom message to the default assertEqual error message + self.longMessage = True # pylint: disable=invalid-name + for field in expectations: + self.assertEqual( + expectations[field], + block_structure.get_transformer_block_field(usage_key, transformer_class, field), + msg=u'in {} and field {}'.format(transformer_class, repr(field)), + ) + + def build_course_with_problems(self, data='', metadata=None): + """ + Create a test course with the requested problem `data` and `metadata` values. + + Appropriate defaults are provided when either argument is omitted. + """ + metadata = metadata or self.problem_metadata + + # Special structure-related keys start with '#'. The rest get passed as + # kwargs to Factory.create. See docstring at + # `CourseStructureTestCase.build_course` for details. + return self.build_course([ + { + u'org': u'GradesTestOrg', + u'course': u'GB101', + u'run': u'cannonball', + u'metadata': {u'format': u'homework'}, + u'#type': u'course', + u'#ref': u'course', + u'#children': [ + { + u'metadata': metadata, + u'#type': u'problem', + u'#ref': u'problem', + u'data': data, + } + ] + } + ]) + + def test_ungraded_block_collection(self): + blocks = self.build_course_with_problems() + block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers) + self.assert_collected_xblock_fields( + block_structure, + blocks[u'course'].location, + weight=None, + graded=False, + has_score=False, + due=None, + format=u'homework', + ) + self.assert_collected_transformer_block_fields( + block_structure, + blocks[u'course'].location, + self.TRANSFORMER_CLASS_TO_TEST, + max_score=None, + ) + + def test_grades_collected_basic(self): + + blocks = self.build_course_with_problems() + block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers) + + self.assert_collected_xblock_fields( + block_structure, + blocks[u'problem'].location, + weight=self.problem_metadata[u'weight'], + graded=self.problem_metadata[u'graded'], + has_score=True, + due=self.problem_metadata[u'due'], + format=None, + ) + + def test_collecting_staff_only_problem(self): + # Demonstrate that the problem data can by collected by the SystemUser + # even if the block has access restrictions placed on it. + problem_metadata = { + u'graded': True, + u'weight': 1, + u'due': datetime.datetime(2016, 10, 16, 0, 4, 0, tzinfo=pytz.utc), + u'visible_to_staff_only': True, + } + + blocks = self.build_course_with_problems(metadata=problem_metadata) + block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers) + + self.assert_collected_xblock_fields( + block_structure, + blocks[u'problem'].location, + weight=problem_metadata[u'weight'], + graded=problem_metadata[u'graded'], + has_score=True, + due=problem_metadata[u'due'], + format=None, + ) + + def test_max_score_collection(self): + problem_data = u''' + + + + + + ''' + + blocks = self.build_course_with_problems(data=problem_data) + block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers) + + self.assert_collected_transformer_block_fields( + block_structure, + blocks[u'problem'].location, + self.TRANSFORMER_CLASS_TO_TEST, + max_score=1, + ) + + def test_max_score_for_multiresponse_problem(self): + problem_data = u''' + + + + + + + + + ''' + + blocks = self.build_course_with_problems(problem_data) + block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers) + + self.assert_collected_transformer_block_fields( + block_structure, + blocks[u'problem'].location, + self.TRANSFORMER_CLASS_TO_TEST, + max_score=2, + ) + + +class MultiProblemModulestoreAccessTestCase(CourseStructureTestCase, SharedModuleStoreTestCase): + """ + Test mongo usage in GradesTransformer. + """ + + TRANSFORMER_CLASS_TO_TEST = GradesTransformer + + def setUp(self): + super(MultiProblemModulestoreAccessTestCase, self).setUp() + password = u'test' + self.student = UserFactory.create(is_staff=False, username=u'test_student', password=password) + self.client.login(username=self.student.username, password=password) + + def test_modulestore_performance(self): + """ + Test that a constant number of mongo calls are made regardless of how + many grade-related blocks are in the course. + """ + course = [ + { + u'org': u'GradesTestOrg', + u'course': u'GB101', + u'run': u'cannonball', + u'metadata': {u'format': u'homework'}, + u'#type': u'course', + u'#ref': u'course', + u'#children': [], + }, + ] + for problem_number in xrange(random.randrange(10, 20)): + course[0][u'#children'].append( + { + u'metadata': { + u'graded': True, + u'weight': 1, + u'due': datetime.datetime(2099, 3, 15, 12, 30, 0, tzinfo=pytz.utc), + }, + u'#type': u'problem', + u'#ref': u'problem_{}'.format(problem_number), + u'data': u''' + + + + + '''.format(number=problem_number), + } + ) + blocks = self.build_course(course) + _get_cache().clear() + with check_mongo_calls(2): + get_course_blocks(self.student, blocks[u'course'].location, self.transformers) diff --git a/lms/djangoapps/courseware/transformers/__init__.py b/lms/djangoapps/courseware/transformers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/courseware/transformers/grades.py b/lms/djangoapps/courseware/transformers/grades.py new file mode 100644 index 0000000000..e53dad93b5 --- /dev/null +++ b/lms/djangoapps/courseware/transformers/grades.py @@ -0,0 +1,102 @@ +""" +Grades Transformer +""" +from django.test.client import RequestFactory + +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer +from openedx.core.djangoapps.util.user_utils import SystemUser +from .. import module_render +from courseware.model_data import FieldDataCache + + +class GradesTransformer(BlockStructureTransformer): + """ + The GradesTransformer collects grading information and stores it on + the block structure. + + No runtime transformations are performed. + + The following values are stored as xblock_fields on their respective blocks in the + block structure: + + due: (datetime) when the problem is due. + format: (string) what type of problem it is + graded: (boolean) + has_score: (boolean) + weight: (numeric) + + Additionally, the following value is calculated and stored as a transformer_block_field + for each block: + + max_score: (numeric) + """ + VERSION = 1 + FIELDS_TO_COLLECT = [u'due', u'format', u'graded', u'has_score', u'weight'] + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return u'grades' + + @classmethod + def collect(cls, block_structure): + """ + Collects any information that's necessary to execute this + transformer's transform method. + """ + block_structure.request_xblock_fields(*cls.FIELDS_TO_COLLECT) + cls._collect_max_scores(block_structure) + + def transform(self, block_structure, usage_context): + """ + Perform no transformations. + """ + pass + + @classmethod + def _collect_max_scores(cls, block_structure): + """ + Collect the `max_score` for every block in the provided `block_structure`. + """ + for module in cls._iter_scorable_xmodules(block_structure): + cls._collect_max_score(block_structure, module) + + @classmethod + def _collect_max_score(cls, block_structure, module): + """ + Collect the `max_score` from the given module, storing it as a + `transformer_block_field` associated with the `GradesTransformer`. + """ + score = module.max_score() + block_structure.set_transformer_block_field(module.location, cls, 'max_score', score) + + @staticmethod + def _iter_scorable_xmodules(block_structure): + """ + Loop through all the blocks locators in the block structure, and retrieve + the module (XModule or XBlock) associated with that locator. + + For implementation reasons, we need to pull the max_score from the + XModule, even though the data is not user specific. Here we bind the + data to a SystemUser. + """ + request = RequestFactory().get('/dummy-collect-max-grades') + user = SystemUser() + request.user = user + request.session = {} + root_block = block_structure.get_xblock(block_structure.root_block_usage_key) + course_key = block_structure.root_block_usage_key.course_key + cache = FieldDataCache.cache_for_descriptor_descendents( + course_id=course_key, + user=request.user, + descriptor=root_block, + descriptor_filter=lambda descriptor: descriptor.has_score, + ) + for block_locator in block_structure.post_order_traversal(): + block = block_structure.get_xblock(block_locator) + if getattr(block, 'has_score', False): + module = module_render.get_module_for_descriptor(user, request, block, cache, course_key) + yield module diff --git a/lms/djangoapps/courseware/user_state_client.py b/lms/djangoapps/courseware/user_state_client.py index 57d09c1ec7..ec5bbee7c4 100644 --- a/lms/djangoapps/courseware/user_state_client.py +++ b/lms/djangoapps/courseware/user_state_client.py @@ -193,6 +193,11 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient): else: user = User.objects.get(username=username) + if user.is_anonymous(): + # Anonymous users cannot be persisted to the database, so let's just use + # what we have. + return + evt_time = time() for usage_key, state in block_keys_to_state.items(): diff --git a/openedx/core/djangoapps/util/tests/__init__.py b/openedx/core/djangoapps/util/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/util/tests/test_user_utils.py b/openedx/core/djangoapps/util/tests/test_user_utils.py new file mode 100644 index 0000000000..7d48cc39c5 --- /dev/null +++ b/openedx/core/djangoapps/util/tests/test_user_utils.py @@ -0,0 +1,27 @@ +"""Tests for util.request module.""" + +import unittest + +from django.contrib.auth.models import AnonymousUser +from ..user_utils import SystemUser + + +class SystemUserTestCase(unittest.TestCase): + """ Tests for response-related utility functions """ + def setUp(self): + super(SystemUserTestCase, self).setUp() + self.sysuser = SystemUser() + + def test_system_user_is_anonymous(self): + self.assertIsInstance(self.sysuser, AnonymousUser) + self.assertTrue(self.sysuser.is_anonymous()) + self.assertIsNone(self.sysuser.id) + + def test_system_user_has_custom_unicode_representation(self): + self.assertNotEqual(unicode(self.sysuser), unicode(AnonymousUser())) + + def test_system_user_is_not_staff(self): + self.assertFalse(self.sysuser.is_staff) + + def test_system_user_is_not_superuser(self): + self.assertFalse(self.sysuser.is_superuser) diff --git a/openedx/core/djangoapps/util/user_utils.py b/openedx/core/djangoapps/util/user_utils.py new file mode 100644 index 0000000000..18ecb9f578 --- /dev/null +++ b/openedx/core/djangoapps/util/user_utils.py @@ -0,0 +1,18 @@ +""" +Custom user-related utility code. +""" + +from django.contrib.auth.models import AnonymousUser + + +class SystemUser(AnonymousUser): + """ + A User that can act on behalf of system actions, when a user object is + needed, but no real user exists. + + Like the AnonymousUser, this User is not represented in the database, and + has no primary key. + """ + # pylint: disable=abstract-method + def __unicode__(self): + return u'SystemUser'