Merge pull request #12396 from edx/cdyer/grading-transformer

Add transformer to collect grades data

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
 
 

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))

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='<problem></problem>', 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'''
+            <problem>
+                <numericalresponse answer="2">
+                    <textline label="1+1" trailing_text="%" />
+                </numericalresponse>
+            </problem>
+        '''
+
+        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'''
+            <problem>
+                <numericalresponse answer="27">
+                    <textline label="3^3" />
+                </numericalresponse>
+                <numericalresponse answer="13.5">
+                    <textline label="and then half of that?" />
+                </numericalresponse>
+            </problem>
+        '''
+
+        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'''
+                        <problem>
+                            <numericalresponse answer="{number}">
+                                <textline label="1*{number}" />
+                            </numericalresponse>
+                        </problem>'''.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)

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

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():

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)

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'