Merge pull request #8391 from edx/alawibaba/plat5892

First draft.

lms/djangoapps/courseware/management/commands/remove_input_state.py

@@ -11,7 +11,8 @@ from optparse import make_option
 from django.core.management.base import BaseCommand, CommandError
 from django.db import transaction
 
-from courseware.models import StudentModule, StudentModuleHistory
+from courseware.models import StudentModule
+from courseware.user_state_client import DjangoXBlockUserStateClient
 
 LOG = logging.getLogger(__name__)
 
@@ -66,13 +67,15 @@ class Command(BaseCommand):
             if student_module_id == 'id':
                 continue
             try:
-                module = StudentModule.objects.get(id=student_module_id)
+                module = StudentModule.objects.select_related('student').get(id=student_module_id)
             except StudentModule.DoesNotExist:
                 LOG.error(u"Unable to find student module with id = %s: skipping... ", student_module_id)
                 continue
             self.remove_studentmodule_input_state(module, save_changes)
 
-            hist_modules = StudentModuleHistory.objects.filter(student_module_id=student_module_id)
+            user_state_client = DjangoXBlockUserStateClient()
+            hist_modules = user_state_client.get_history(module.student.username, module.module_state_key)
+
             for hist_module in hist_modules:
                 self.remove_studentmodulehistory_input_state(hist_module, save_changes)
 

lms/djangoapps/courseware/model_data.py

@@ -419,6 +419,7 @@ class UserStateCache(object):
                 pending_updates
             )
         except DatabaseError:
+            log.exception("Saving user state failed for %s", self.user.username)
             raise KeyValueMultiSaveError([])
         finally:
             self._cache.update(pending_updates)

lms/djangoapps/courseware/user_state_client.py

@@ -11,9 +11,10 @@ try:
 except ImportError:
     import json
 
+from django.contrib.auth.models import User
 from xblock.fields import Scope, ScopeBase
 from xblock_user_state.interface import XBlockUserStateClient
-from courseware.models import StudentModule
+from courseware.models import StudentModule, StudentModuleHistory
 from contracts import contract, new_contract
 from opaque_keys.edx.keys import UsageKey
 
@@ -43,7 +44,13 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
         """
         pass
 
-    def __init__(self, user):
+    def __init__(self, user=None):
+        """
+        Arguments:
+            user (:class:`~User`): An already-loaded django user. If this user matches the username
+                supplied to `set_many`, then that will reduce the number of queries made to store
+                the user state.
+        """
         self.user = user
 
     @contract(
@@ -68,7 +75,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
         Raises:
             DoesNotExist if no entry is found.
         """
-        assert self.user.username == username
         try:
             _usage_key, state = next(self.get_many(username, [block_key], scope, fields=fields))
         except StopIteration:
@@ -87,7 +93,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
             state (dict): A dictionary mapping field names to values
             scope (Scope): The scope to load data from
         """
-        assert self.user.username == username
         self.set_many(username, {block_key: state}, scope)
 
     @contract(
@@ -106,7 +111,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
             scope (Scope): The scope to delete data from
             fields: A list of fields to delete. If None, delete all stored fields.
         """
-        assert self.user.username == username
         return self.delete_many(username, [block_key], scope, fields=fields)
 
     @contract(
@@ -182,7 +186,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
             (UsageKey, field_state) tuples for each specified UsageKey in block_keys.
             field_state is a dict mapping field names to values.
         """
-        assert self.user.username == username
         if scope != Scope.user_state:
             raise ValueError("Only Scope.user_state is supported, not {}".format(scope))
 
@@ -207,7 +210,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
                 :meth:`delete` or :meth:`delete_many`.
             scope (Scope): The scope to load data from
         """
-        assert self.user.username == username
         if scope != Scope.user_state:
             raise ValueError("Only Scope.user_state is supported")
 
@@ -215,9 +217,14 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
         # that were queried in get_many) so that if the score has
         # been changed by some other piece of the code, we don't overwrite
         # that score.
+        if self.user.username == username:
+            user = self.user
+        else:
+            user = User.objects.get(username=username)
+
         for usage_key, state in block_keys_to_state.items():
             student_module, created = StudentModule.objects.get_or_create(
-                student=self.user,
+                student=user,
                 course_id=usage_key.course_key,
                 module_state_key=usage_key,
                 defaults={
@@ -252,7 +259,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
             scope (Scope): The scope to delete data from
             fields: A list of fields to delete. If None, delete all stored fields.
         """
-        assert self.user.username == username
         if scope != Scope.user_state:
             raise ValueError("Only Scope.user_state is supported")
 
@@ -291,7 +297,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
 
         Yields: tuples of (block, field_name, modified_date) for each selected field.
         """
-        assert self.user.username == username
         if scope != Scope.user_state:
             raise ValueError("Only Scope.user_state is supported")
 
@@ -303,12 +308,41 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
             for field in json.loads(student_module.state):
                 yield (usage_key, field, student_module.modified)
 
+    @contract(username="basestring", block_key=UsageKey, scope=ScopeBase)
     def get_history(self, username, block_key, scope=Scope.user_state):
-        """We don't guarantee that history for many blocks will be fast."""
-        assert self.user.username == username
+        """
+        Retrieve history of state changes for a given block for a given
+        student.  We don't guarantee that history for many blocks will be fast.
+
+        Arguments:
+            username: The name of the user whose history should be retrieved
+            block_key (UsageKey): The UsageKey identifying which xblock state to update.
+            scope (Scope): The scope to load data from
+        """
+
         if scope != Scope.user_state:
             raise ValueError("Only Scope.user_state is supported")
-        raise NotImplementedError()
+        student_modules = list(
+            student_module
+            for student_module, usage_id
+            in self._get_student_modules(username, [block_key])
+        )
+        if len(student_modules) == 0:
+            raise self.DoesNotExist()
+
+        history_entries = StudentModuleHistory.objects.filter(
+            student_module__in=student_modules
+        ).order_by('-id')
+
+        # If no history records exist, let's force a save to get history started.
+        if not history_entries:
+            for student_module in student_modules:
+                student_module.save()
+            history_entries = StudentModuleHistory.objects.filter(
+                student_module__in=student_modules
+            ).order_by('-id')
+
+        return history_entries
 
     def iter_all_for_block(self, block_key, scope=Scope.user_state, batch_size=None):
         """

lms/djangoapps/courseware/views.py

@@ -47,7 +47,7 @@ from .entrance_exams import (
     user_must_complete_entrance_exam,
     user_has_passed_entrance_exam
 )
-from courseware.models import StudentModule, StudentModuleHistory
+from courseware.user_state_client import DjangoXBlockUserStateClient
 from course_modes.models import CourseMode
 
 from open_ended_grading import open_ended_notifications
@@ -1109,34 +1109,18 @@ def submission_history(request, course_id, student_username, location):
     if (student_username != request.user.username) and (not staff_access):
         raise PermissionDenied
 
+    user_state_client = DjangoXBlockUserStateClient()
     try:
-        student = User.objects.get(username=student_username)
-        student_module = StudentModule.objects.get(
-            course_id=course_key,
-            module_state_key=usage_key,
-            student_id=student.id
-        )
-    except User.DoesNotExist:
-        return HttpResponse(escape(_(u'User {username} does not exist.').format(username=student_username)))
-    except StudentModule.DoesNotExist:
+        history_entries = user_state_client.get_history(student_username, usage_key)
+    except DjangoXBlockUserStateClient.DoesNotExist:
         return HttpResponse(escape(_(u'User {username} has never accessed problem {location}').format(
             username=student_username,
             location=location
         )))
-    history_entries = StudentModuleHistory.objects.filter(
-        student_module=student_module
-    ).order_by('-id')
-
-    # If no history records exist, let's force a save to get history started.
-    if not history_entries:
-        student_module.save()
-        history_entries = StudentModuleHistory.objects.filter(
-            student_module=student_module
-        ).order_by('-id')
 
     context = {
         'history_entries': history_entries,
-        'username': student.username,
+        'username': student_username,
         'location': location,
         'course_id': course_key.to_deprecated_string()
     }