From 704ae7139a621344f37d3ef5eeab223bba834b15 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 27 Jul 2015 14:31:06 -0400 Subject: [PATCH 1/5] Add a subclass-implementation of the UserStateClient tests --- lms/djangoapps/courseware/model_data.py | 4 +- .../tests/test_user_state_client.py | 67 +++++++++++++++++++ requirements/edx/github.txt | 2 +- 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 lms/djangoapps/courseware/tests/test_user_state_client.py diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 64d915a3e9..0fd45a183e 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -392,11 +392,11 @@ class UserStateCache(object): Returns: datetime if there was a modified date, or None otherwise """ - return self._client.get_mod_date( + return self._client.get( self.user.username, kvs_key.block_scope_id, fields=[kvs_key.field_name], - ).get(kvs_key.field_name) + ).updated @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/requirements/edx/github.txt b/requirements/edx/github.txt index fb11e60171..95ff5c15ee 100644 --- a/requirements/edx/github.txt +++ b/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 From 53b37e7412848e2eae3ec8566f507b2de8fdf32d Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 27 Jul 2015 14:31:47 -0400 Subject: [PATCH 2/5] Remove extraneous documentation, contracts, and method definitions --- .../courseware/user_state_client.py | 49 +------------------ 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/lms/djangoapps/courseware/user_state_client.py b/lms/djangoapps/courseware/user_state_client.py index 3cb7731603..f268c7617f 100644 --- a/lms/djangoapps/courseware/user_state_client.py +++ b/lms/djangoapps/courseware/user_state_client.py @@ -53,53 +53,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. @@ -157,7 +110,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: From aa374ca12ab9ed424c91d02e7d477e1a6953f8b8 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 27 Jul 2015 14:36:02 -0400 Subject: [PATCH 3/5] Make DjangoXBlockUserStateClient pass semantic tests This required the following changes to the DjangoXBlockUserStateClient semantics: 1) Changes get/get_many to return XBlockUserState tuples, rather than state dictionaries or (block_key, state) tuples. 2) Raises DoesNotExist if get_history is called on an XBlock that has had no data saved to it. 3) Returns XBlockUserState tuples as the results of get_history. --- .../management/commands/remove_input_state.py | 21 +-- lms/djangoapps/courseware/model_data.py | 17 ++- .../courseware/user_state_client.py | 131 +++++++++--------- lms/djangoapps/courseware/views.py | 28 +++- .../courseware/submission_history.html | 8 +- 5 files changed, 119 insertions(+), 86 deletions(-) diff --git a/lms/djangoapps/courseware/management/commands/remove_input_state.py b/lms/djangoapps/courseware/management/commands/remove_input_state.py index 988dcc3507..d987a3173a 100644 --- a/lms/djangoapps/courseware/management/commands/remove_input_state.py +++ b/lms/djangoapps/courseware/management/commands/remove_input_state.py @@ -76,16 +76,19 @@ class Command(BaseCommand): 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) + try: + 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 - ) + 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 + ) + except DjangoXBlockUserStateClient.DoesNotExist: + LOG.info("No history entries found for %s", module.module_state_key) @transaction.autocommit def remove_studentmodule_input_state(self, module, save_changes): diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 0fd45a183e..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( - self.user.username, - kvs_key.block_scope_id, - fields=[kvs_key.field_name], - ).updated + 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/user_state_client.py b/lms/djangoapps/courseware/user_state_client.py index f268c7617f..008dd914dc 100644 --- a/lms/djangoapps/courseware/user_state_client.py +++ b/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,26 +68,6 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient): """ self.user = user - 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``. @@ -101,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 @@ -119,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): """ @@ -143,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) @@ -185,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: @@ -193,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: @@ -243,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() - return history_entries + 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 + ) + + 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): """ diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index ba2673c3a9..f95bcc5d3f 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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() diff --git a/lms/templates/courseware/submission_history.html b/lms/templates/courseware/submission_history.html index b4ce5ed2a6..b89fb420f9 100644 --- a/lms/templates/courseware/submission_history.html +++ b/lms/templates/courseware/submission_history.html @@ -1,13 +1,13 @@ <% import json %>

${username | h} > ${course_id | h} > ${location | h}

-% for i, entry in enumerate(history_entries): +% for i, (entry, score) in enumerate(zip(history_entries, scores)):
-#${len(history_entries) - i}: ${entry.created} (${TIME_ZONE} time)
-Score: ${entry.grade} / ${entry.max_grade} +#${len(history_entries) - i}: ${entry.updated} (${TIME_ZONE} time)
+Score: ${score.grade} / ${score.max_grade} 
-${json.dumps(json.loads(entry.state), indent=2, sort_keys=True) | h}
+${json.dumps(entry.state, indent=2, sort_keys=True) | h}
% endfor From 6ba611cb5fa42bc9a0045bd8a6ece1ddf6dfdbcc Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 28 Jul 2015 14:43:27 -0400 Subject: [PATCH 4/5] Remove remove_input_state.py, as it was one-time fix code, and is not worth porting to the new interface --- .../management/commands/remove_input_state.py | 164 ------------------ 1 file changed, 164 deletions(-) delete mode 100644 lms/djangoapps/courseware/management/commands/remove_input_state.py 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 d987a3173a..0000000000 --- a/lms/djangoapps/courseware/management/commands/remove_input_state.py +++ /dev/null @@ -1,164 +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) - - try: - 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 - ) - except DjangoXBlockUserStateClient.DoesNotExist: - LOG.info("No history entries found for %s", module.module_state_key) - - @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 - ) From 83c10331fad1986d3b6e7357814b8c3d80a2dd59 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 29 Jul 2015 16:03:05 -0400 Subject: [PATCH 5/5] Add a test of submission history display --- lms/djangoapps/courseware/tests/test_views.py | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) 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('