diff --git a/lms/djangoapps/courseware/management/commands/remove_input_state.py b/lms/djangoapps/courseware/management/commands/remove_input_state.py deleted file mode 100644 index 988dcc3507..0000000000 --- a/lms/djangoapps/courseware/management/commands/remove_input_state.py +++ /dev/null @@ -1,161 +0,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 - ) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 64d915a3e9..c7275831c7 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/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): diff --git a/lms/djangoapps/courseware/tests/test_user_state_client.py b/lms/djangoapps/courseware/tests/test_user_state_client.py new file mode 100644 index 0000000000..11b98f9e2d --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_user_state_client.py @@ -0,0 +1,67 @@ +""" +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 diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 4ece972ea6..d8891170cd 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/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('