Skip to content

E
edx-platform
Commit ffc8a117 by John Eskew
Options

Merge pull request #9092 from cpennington/user-state-client-tests 

User state client tests
parents eaeb3256 83c10331
Showing with 224 additions and 287 deletions
lms/djangoapps/courseware/management/commands/remove_input_state.py deleted 100644 → 0
'''
This is a one-off command aimed at fixing a temporary problem encountered where input_state was added to
the same dict object in capa problems, so was accumulating. The fix is simply to remove input_state entry
from state for all problems in the affected date range.
'''
import json
import logging
from optparse import make_option
from django.core.management.base import BaseCommand, CommandError
from django.db import transaction
from courseware.models import StudentModule
from courseware.user_state_client import DjangoXBlockUserStateClient
LOG = logging.getLogger(__name__)
class Command(BaseCommand):
'''
The fix here is to remove the "input_state" entry in the StudentModule objects of any problems that
contain them. No problem is yet making use of this, and the code should do the right thing if it's
missing (by recreating an empty dict for its value).
To narrow down the set of problems that might need fixing, the StudentModule
objects to be checked is filtered down to those:
created < '2013-03-29 16:30:00' (the problem must have been answered before the buggy code was reverted,
on Prod and Edge)
modified > '2013-03-28 22:00:00' (the problem must have been visited after the bug was introduced
on Prod and Edge)
state like '%input_state%' (the problem must have "input_state" set).
This filtering is done on the production database replica, so that the larger select queries don't lock
the real production database. The list of id values for Student Modules is written to a file, and the
file is passed into this command. The sql file passed to mysql contains:
select sm.id from courseware_studentmodule sm
where sm.modified > "2013-03-28 22:00:00"
and sm.created < "2013-03-29 16:30:00"
and sm.state like "%input_state%"
and sm.module_type = 'problem';
'''
num_visited = 0
num_changed = 0
num_hist_visited = 0
num_hist_changed = 0
option_list = BaseCommand.option_list + (
make_option('--save',
action='store_true',
dest='save_changes',
default=False,
help='Persist the changes that were encountered. If not set, no changes are saved.'),
)
def fix_studentmodules_in_list(self, save_changes, idlist_path):
'''Read in the list of StudentModule objects that might need fixing, and then fix each one'''
# open file and read id values from it:
for line in open(idlist_path, 'r'):
student_module_id = line.strip()
# skip the header, if present:
if student_module_id == 'id':
continue
try:
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)
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)
if self.num_visited % 1000 == 0:
LOG.info(" Progress: updated %s of %s student modules", self.num_changed, self.num_visited)
LOG.info(
" Progress: updated %s of %s student history modules",
self.num_hist_changed,
self.num_hist_visited
)
@transaction.autocommit
def remove_studentmodule_input_state(self, module, save_changes):
''' Fix the grade assigned to a StudentModule'''
module_state = module.state
if module_state is None:
# not likely, since we filter on it. But in general...
LOG.info(
"No state found for %s module %s for student %s in course %s",
module.module_type, module.module_state_key,
module.student.username, module.course_id
)
return
state_dict = json.loads(module_state)
self.num_visited += 1
if 'input_state' not in state_dict:
pass
elif save_changes:
# make the change and persist
del state_dict['input_state']
module.state = json.dumps(state_dict)
module.save()
self.num_changed += 1
else:
# don't make the change, but increment the count indicating the change would be made
self.num_changed += 1
@transaction.autocommit
def remove_studentmodulehistory_input_state(self, module, save_changes):
''' Fix the grade assigned to a StudentModule'''
module_state = module.state
if module_state is None:
# not likely, since we filter on it. But in general...
LOG.info(
"No state found for %s module %s for student %s in course %s",
module.module_type, module.module_state_key,
module.student.username, module.course_id
)
return
state_dict = json.loads(module_state)
self.num_hist_visited += 1
if 'input_state' not in state_dict:
pass
elif save_changes:
# make the change and persist
del state_dict['input_state']
module.state = json.dumps(state_dict)
module.save()
self.num_hist_changed += 1
else:
# don't make the change, but increment the count indicating the change would be made
self.num_hist_changed += 1
def handle(self, *args, **options):
'''Handle management command request'''
if len(args) != 1:
raise CommandError("missing idlist file")
idlist_path = args[0]
save_changes = options['save_changes']
LOG.info("Starting run: reading from idlist file %s; save_changes = %s", idlist_path, save_changes)
self.fix_studentmodules_in_list(save_changes, idlist_path)
LOG.info("Finished run: updating %s of %s student modules", self.num_changed, self.num_visited)
LOG.info(
"Finished run: updating %s of %s student history modules",
self.num_hist_changed,
self.num_hist_visited
)
lms/djangoapps/courseware/model_data.py
......@@ -368,8 +368,8 @@ class UserStateCache(object):
self.user.username,
_all_usage_keys(xblocks, aside_types),
)
for usage_key, field_state in block_field_state:
self._cache[usage_key] = field_state
for user_state in block_field_state:
self._cache[user_state.block_key] = user_state.state
@contract(kvs_key=DjangoKeyValueStore.Key)
def set(self, kvs_key, value):
......@@ -392,11 +392,14 @@ class UserStateCache(object):
Returns: datetime if there was a modified date, or None otherwise
"""
return self._client.get_mod_date(
self.user.username,
kvs_key.block_scope_id,
fields=[kvs_key.field_name],
).get(kvs_key.field_name)
try:
return self._client.get(
self.user.username,
kvs_key.block_scope_id,
fields=[kvs_key.field_name],
).updated
except self._client.DoesNotExist:
return None
@contract(kv_dict="dict(DjangoKeyValueStore_Key: *)")
def set_many(self, kv_dict):
......
lms/djangoapps/courseware/tests/test_user_state_client.py 0 → 100644
"""
Black-box tests of the DjangoUserStateClient against the semantics
defined in edx_user_state_client.
"""
from collections import defaultdict
from unittest import skip
from django.test import TestCase
from edx_user_state_client.tests import UserStateClientTestBase
from courseware.user_state_client import DjangoXBlockUserStateClient
from courseware.tests.factories import UserFactory
class TestDjangoUserStateClient(UserStateClientTestBase, TestCase):
"""
Tests of the DjangoUserStateClient backend.
"""
__test__ = True
def _user(self, user_idx):
return self.users[user_idx].username
def _block_type(self, block): # pylint: disable=unused-argument
# We only record block state history in DjangoUserStateClient
# when the block type is 'problem'
return 'problem'
def setUp(self):
super(TestDjangoUserStateClient, self).setUp()
self.client = DjangoXBlockUserStateClient()
self.users = defaultdict(UserFactory.create)
# We're skipping these tests because the iter_all_by_block and iter_all_by_course
# are not implemented in the DjangoXBlockUserStateClient
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_blocks_deleted_block(self):
pass
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_blocks_empty(self):
pass
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_blocks_many_users(self):
pass
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_blocks_single_user(self):
pass
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_course_deleted_block(self):
pass
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_course_empty(self):
pass
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_course_single_user(self):
pass
@skip("Not supported by DjangoXBlockUserStateClient")
def test_iter_course_many_users(self):
pass
lms/djangoapps/courseware/tests/test_views.py
......@@ -8,6 +8,7 @@ import ddt
import json
import unittest
from datetime import datetime
from HTMLParser import HTMLParser
from nose.plugins.attrib import attr
from django.conf import settings
......@@ -30,8 +31,10 @@ from certificates import api as certs_api
from certificates.models import CertificateStatuses, CertificateGenerationConfiguration
from certificates.tests.factories import GeneratedCertificateFactory
from course_modes.models import CourseMode
from courseware.model_data import set_score
from courseware.testutils import RenderXBlockTestMixin
from courseware.tests.factories import StudentModuleFactory
from courseware.user_state_client import DjangoXBlockUserStateClient
from edxmako.tests import mako_middleware_process_request
from student.models import CourseEnrollment
from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory
......@@ -524,6 +527,51 @@ class ViewsTestCase(ModuleStoreTestCase):
response = self.client.get(url)
self.assertFalse('<script>' in response.content)
def test_submission_history_contents(self):
# log into a staff account
admin = AdminFactory.create()
self.client.login(username=admin.username, password='test')
usage_key = self.course_key.make_usage_key('problem', 'test-history')
state_client = DjangoXBlockUserStateClient(admin)
# store state via the UserStateClient
state_client.set(
username=admin.username,
block_key=usage_key,
state={'field_a': 'x', 'field_b': 'y'}
)
set_score(admin.id, usage_key, 0, 3)
state_client.set(
username=admin.username,
block_key=usage_key,
state={'field_a': 'a', 'field_b': 'b'}
)
set_score(admin.id, usage_key, 3, 3)
url = reverse('submission_history', kwargs={
'course_id': unicode(self.course_key),
'student_username': admin.username,
'location': unicode(usage_key),
})
response = self.client.get(url)
response_content = HTMLParser().unescape(response.content)
# We have update the state 4 times: twice to change content, and twice
# to set the scores. We'll check that the identifying content from each is
# displayed (but not the order), and also the indexes assigned in the output
# #1 - #4
self.assertIn('#1', response_content)
self.assertIn(json.dumps({'field_a': 'a', 'field_b': 'b'}, sort_keys=True, indent=2), response_content)
self.assertIn("Score: 0.0 / 3.0", response_content)
self.assertIn(json.dumps({'field_a': 'x', 'field_b': 'y'}, sort_keys=True, indent=2), response_content)
self.assertIn("Score: 3.0 / 3.0", response_content)
self.assertIn('#4', response_content)
def _load_mktg_about(self, language=None, org=None):
"""Retrieve the marketing about button (iframed into the marketing site)
and return the HTTP response.
......
lms/djangoapps/courseware/user_state_client.py
......@@ -13,7 +13,7 @@ except ImportError:
from django.contrib.auth.models import User
from xblock.fields import Scope, ScopeBase
from edx_user_state_client.interface import XBlockUserStateClient
from edx_user_state_client.interface import XBlockUserStateClient, XBlockUserState
from courseware.models import StudentModule, StudentModuleHistory
from contracts import contract, new_contract
from opaque_keys.edx.keys import UsageKey
......@@ -24,6 +24,21 @@ new_contract('UsageKey', UsageKey)
class DjangoXBlockUserStateClient(XBlockUserStateClient):
"""
An interface that uses the Django ORM StudentModule as a backend.
A note on the format of state storage:
The state for an xblock is stored as a serialized JSON dictionary. The model
field that it is stored in can also take on a value of ``None``. To preserve
existing analytic uses, we will preserve the following semantics:
A state of ``None`` means that the user hasn't ever looked at the xblock.
A state of ``"{}"`` means that the XBlock has at some point stored state for
the current user, but that that state has been deleted.
Otherwise, the dictionary contains all data stored for the user.
None of these conditions should violate the semantics imposed by
XBlockUserStateClient (for instance, once all fields have been deleted from
an XBlock for a user, the state will be listed as ``None`` by :meth:`get_history`,
even though the actual stored state in the database will be ``"{}"``).
"""
class ServiceUnavailable(XBlockUserStateClient.ServiceUnavailable):
......@@ -53,73 +68,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
"""
self.user = user
def get(self, username, block_key, scope=Scope.user_state, fields=None):
"""
Retrieve the stored XBlock state for a single xblock usage.
Arguments:
username: The name of the user whose state should be retrieved
block_key (UsageKey): The UsageKey identifying which xblock state to load.
scope (Scope): The scope to load data from
fields: A list of field values to retrieve. If None, retrieve all stored fields.
Returns:
dict: A dictionary mapping field names to values
Raises:
DoesNotExist if no entry is found.
"""
try:
_usage_key, state = next(self.get_many(username, [block_key], scope, fields=fields))
except StopIteration:
raise self.DoesNotExist()
return state
def set(self, username, block_key, state, scope=Scope.user_state):
"""
Set fields for a particular XBlock.
Arguments:
username: The name of the user whose state should be retrieved
block_key (UsageKey): The UsageKey identifying which xblock state to update.
state (dict): A dictionary mapping field names to values
scope (Scope): The scope to load data from
"""
self.set_many(username, {block_key: state}, scope)
def delete(self, username, block_key, scope=Scope.user_state, fields=None):
"""
Delete the stored XBlock state for a single xblock usage.
Arguments:
username: The name of the user whose state should be deleted
block_key (UsageKey): The UsageKey identifying which xblock state to delete.
scope (Scope): The scope to delete data from
fields: A list of fields to delete. If None, delete all stored fields.
"""
return self.delete_many(username, [block_key], scope, fields=fields)
def get_mod_date(self, username, block_key, scope=Scope.user_state, fields=None):
"""
Get the last modification date for fields from the specified blocks.
Arguments:
username: The name of the user whose state should be deleted
block_key (UsageKey): The UsageKey identifying which xblock modification dates to retrieve.
scope (Scope): The scope to retrieve from.
fields: A list of fields to query. If None, delete all stored fields.
Specific implementations are free to return the same modification date
for all fields, if they don't store changes individually per field.
Implementations may omit fields for which data has not been stored.
Returns: list a dict of {field_name: modified_date} for each selected field.
"""
results = self.get_mod_date_many(username, [block_key], scope, fields=fields)
return {
field: date for (_, field, date) in results
}
def _get_student_modules(self, username, block_keys):
"""
Retrieve the :class:`~StudentModule`s for the supplied ``username`` and ``block_keys``.
......@@ -148,7 +96,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
def get_many(self, username, block_keys, scope=Scope.user_state, fields=None):
"""
Retrieve the stored XBlock state for a single xblock usage.
Retrieve the stored XBlock state for the specified XBlock usages.
Arguments:
username: The name of the user whose state should be retrieved
......@@ -157,7 +105,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
fields: A list of field values to retrieve. If None, retrieve all stored fields.
Yields:
(UsageKey, field_state) tuples for each specified UsageKey in block_keys.
XBlockUserState tuples for each specified UsageKey in block_keys.
field_state is a dict mapping field names to values.
"""
if scope != Scope.user_state:
......@@ -166,10 +114,22 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
modules = self._get_student_modules(username, block_keys)
for module, usage_key in modules:
if module.state is None:
state = {}
else:
state = json.loads(module.state)
yield (usage_key, state)
continue
state = json.loads(module.state)
# If the state is the empty dict, then it has been deleted, and so
# conformant UserStateClients should treat it as if it doesn't exist.
if state == {}:
continue
if fields is not None:
state = {
field: state[field]
for field in fields
if field in state
}
yield XBlockUserState(username, usage_key, state, module.modified, scope)
def set_many(self, username, block_keys_to_state, scope=Scope.user_state):
"""
......@@ -190,7 +150,7 @@ 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:
if self.user is not None and self.user.username == username:
user = self.user
else:
user = User.objects.get(username=username)
......@@ -232,7 +192,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
student_modules = self._get_student_modules(username, block_keys)
for student_module, _ in student_modules:
if fields is None:
student_module.state = "{}"
student_module.state = None
else:
current_state = json.loads(student_module.state)
for field in fields:
......@@ -240,44 +200,25 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
del current_state[field]
student_module.state = json.dumps(current_state)
# We just read this object, so we know that we can do an update
student_module.save(force_update=True)
def get_mod_date_many(self, username, block_keys, scope=Scope.user_state, fields=None):
"""
Get the last modification date for fields from the specified blocks.
Arguments:
username: The name of the user whose state should be deleted
block_key (UsageKey): The UsageKey identifying which xblock modification dates to retrieve.
scope (Scope): The scope to retrieve from.
fields: A list of fields to query. If None, delete all stored fields.
Specific implementations are free to return the same modification date
for all fields, if they don't store changes individually per field.
Implementations may omit fields for which data has not been stored.
Yields: tuples of (block, field_name, modified_date) for each selected field.
"""
if scope != Scope.user_state:
raise ValueError("Only Scope.user_state is supported")
student_modules = self._get_student_modules(username, block_keys)
for student_module, usage_key in student_modules:
if student_module.state is None:
continue
for field in json.loads(student_module.state):
yield (usage_key, field, student_module.modified)
def get_history(self, username, block_key, scope=Scope.user_state):
"""
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.
If the specified block doesn't exist, raise :class:`~DoesNotExist`.
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
username: The name of the user whose history should be retrieved.
block_key: The key identifying which xblock history to retrieve.
scope (Scope): The scope to load data from.
Yields:
XBlockUserState entries for each modification to the specified XBlock, from latest
to earliest.
"""
if scope != Scope.user_state:
......@@ -290,19 +231,32 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
if len(student_modules) == 0:
raise self.DoesNotExist()
history_entries = StudentModuleHistory.objects.filter(
history_entries = StudentModuleHistory.objects.prefetch_related('student_module').filter(
student_module__in=student_modules
).order_by('-id')
# If no history records exist, let's force a save to get history started.
# If no history records exist, raise an error
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')
raise self.DoesNotExist()
for history_entry in history_entries:
state = history_entry.state
# If the state is serialized json, then load it
if state is not None:
state = json.loads(state)
# If the state is empty, then for the purposes of `get_history`, it has been
# deleted, and so we list that entry as `None`.
if state == {}:
state = None
block_key = history_entry.student_module.module_state_key
block_key = block_key.map_into_course(
history_entry.student_module.course_id
)
return history_entries
yield XBlockUserState(username, block_key, state, history_entry.created, scope)
def iter_all_for_block(self, block_key, scope=Scope.user_state, batch_size=None):
"""
......
lms/djangoapps/courseware/views.py
......@@ -44,6 +44,7 @@ from openedx.core.djangoapps.credit.api import (
is_user_eligible_for_credit,
is_credit_course
)
from courseware.models import StudentModuleHistory
from courseware.model_data import FieldDataCache, ScoresClient
from .module_render import toc_for_course, get_module_for_descriptor, get_module, get_module_by_usage_id
from .entrance_exams import (
......@@ -1201,15 +1202,40 @@ def submission_history(request, course_id, student_username, location):
user_state_client = DjangoXBlockUserStateClient()
try:
history_entries = user_state_client.get_history(student_username, usage_key)
history_entries = list(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
)))
# This is ugly, but until we have a proper submissions API that we can use to provide
# the scores instead, it will have to do.
scores = list(StudentModuleHistory.objects.filter(
student_module__module_state_key=usage_key
).order_by('-id'))
if len(scores) != len(history_entries):
log.warning(
"Mismatch when fetching scores for student "
"history for course %s, user %s, xblock %s. "
"Matching scores by date for display.",
course_id,
student_username,
location
)
scores_by_date = {
score.modified: score
for score in scores
}
scores = [
scores_by_date[history.updated]
for history in history_entries
]
context = {
'history_entries': history_entries,
'scores': scores,
'username': student_username,
'location': location,
'course_id': course_key.to_deprecated_string()
......
lms/templates/courseware/submission_history.html
<% import json %>
<h3>${username | h} > ${course_id | h} > ${location | h}</h3>
% for i, entry in enumerate(history_entries):
% for i, (entry, score) in enumerate(zip(history_entries, scores)):
<hr/>
<div>
<b>#${len(history_entries) - i}</b>: ${entry.created} (${TIME_ZONE} time)</br>
Score: ${entry.grade} / ${entry.max_grade}
<b>#${len(history_entries) - i}</b>: ${entry.updated} (${TIME_ZONE} time)</br>
Score: ${score.grade} / ${score.max_grade}
<pre>
${json.dumps(json.loads(entry.state), indent=2, sort_keys=True) | h}
${json.dumps(entry.state, indent=2, sort_keys=True) | h}
</pre>
</div>
% endfor
requirements/edx/github.txt
......@@ -55,7 +55,7 @@ git+https://github.com/edx/edx-lint.git@ed8c8d2a0267d4d42f43642d193e25f8bd575d9b
-e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive
-e git+https://github.com/edx/edx-reverification-block.git@485c189f4c5d9ad34e8856e385be546c0ad0a9aa#egg=edx-reverification-block
git+https://github.com/edx/ecommerce-api-client.git@1.1.0#egg=ecommerce-api-client==1.1.0
-e git+https://github.com/edx/edx-user-state-client.git@64a8b603f42669bb7fdca03d364d4e8d3d6ad67d#egg=edx-user-state-client
-e git+https://github.com/edx/edx-user-state-client.git@30c0ad4b9f57f8d48d6943eb585ec8a9205f4469#egg=edx-user-state-client
-e git+https://github.com/edx/edx-proctoring.git@release-2015-07-29#egg=edx-proctoring
# Third Party XBlocks
......
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