From dd5ac4cf34908c2c245b51668bb4455808ead2d6 Mon Sep 17 00:00:00 2001 From: cahrens Date: Tue, 26 Nov 2013 15:02:25 -0500 Subject: [PATCH 1/5] Change video transcripts to use locators instead of locations. Part of STUD-870 --- .../contentstore/tests/test_contentstore.py | 9 +- .../contentstore/tests/test_transcripts.py | 80 +++++++-------- .../contentstore/views/component.py | 10 +- .../contentstore/views/transcripts_ajax.py | 97 ++++++++++--------- .../js/spec/transcripts/file_uploader_spec.js | 2 +- .../spec/transcripts/message_manager_spec.js | 10 +- .../js/spec/transcripts/videolist_spec.js | 8 +- cms/static/js/views/transcripts/editor.js | 18 ++-- .../js/views/transcripts/file_uploader.js | 2 +- .../js/views/transcripts/message_manager.js | 10 +- .../views/transcripts/metadata_videolist.js | 6 +- cms/static/js/views/transcripts/utils.js | 6 +- .../js/transcripts/file-upload.underscore | 2 +- .../messages/transcripts-found.underscore | 2 +- .../messages/transcripts-uploaded.underscore | 2 +- cms/templates/unit.html | 4 +- 16 files changed, 130 insertions(+), 138 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 0aaf2dfb2992..91dce0f6ee7f 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -137,8 +137,7 @@ def check_components_on_page(self, component_types, expected_types): locator = loc_mapper().translate_location(course.location.course_id, descriptor.location, False, True) resp = self.client.get_html(locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) - # TODO: uncomment when video transcripts no longer require IDs. - # _test_no_locations(self, resp) + _test_no_locations(self, resp) for expected in expected_types: self.assertIn(expected, resp.content) @@ -1354,8 +1353,7 @@ def _check_verticals(self, items, course_id): unit_locator = loc_mapper().translate_location(course_id, descriptor.location, False, True) resp = self.client.get_html(unit_locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) - # TODO: uncomment when video transcripts no longer require IDs. - # _test_no_locations(self, resp) + _test_no_locations(self, resp) @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE, MODULESTORE=TEST_MODULESTORE) @@ -1682,8 +1680,7 @@ def test_get_html(page): unit_locator = loc_mapper().translate_location(loc.course_id, unit_location, False, True) resp = self.client.get_html(unit_locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) - # TODO: uncomment when video transcripts no longer require IDs. - # _test_no_locations(self, resp) + _test_no_locations(self, resp) def delete_item(category, name): """ Helper method for testing the deletion of an xblock item. """ diff --git a/cms/djangoapps/contentstore/tests/test_transcripts.py b/cms/djangoapps/contentstore/tests/test_transcripts.py index 4c481383ab5f..f4c7f773adf0 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts.py @@ -60,7 +60,7 @@ def setUp(self): 'type': 'video' } resp = self.client.ajax_post('/xblock', data) - self.item_location = self._get_location(resp) + self.item_locator, self.item_location = self._get_locator(resp) self.assertEqual(resp.status_code, 200) # hI10vDNYz4M - valid Youtube ID with transcripts. @@ -73,10 +73,10 @@ def setUp(self): # Remove all transcripts for current module. self.clear_subs_content() - def _get_location(self, resp): - """ Returns the location (as a string) from the response returned by a create operation. """ + def _get_locator(self, resp): + """ Returns the locator and old-style location (as a string) from the response returned by a create operation. """ locator = json.loads(resp.content).get('locator') - return loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url() + return locator, loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url() def get_youtube_ids(self): """Return youtube speeds and ids.""" @@ -142,7 +142,7 @@ def test_success_video_module_source_subs_uploading(self): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -164,20 +164,20 @@ def test_fail_data_without_id(self): link = reverse('upload_transcripts') resp = self.client.post(link, {'file': self.good_srt_file}) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "id" form data.') + self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "locator" form data.') def test_fail_data_without_file(self): link = reverse('upload_transcripts') - resp = self.client.post(link, {'id': self.item_location}) + resp = self.client.post(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 400) self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "file" form data.') - def test_fail_data_with_bad_location(self): + def test_fail_data_with_bad_locator(self): # Test for raising `InvalidLocationError` exception. link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': 'BAD_LOCATION', + 'locator': 'BAD_LOCATOR', 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -186,13 +186,13 @@ def test_fail_data_with_bad_location(self): }]) }) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") # Test for raising `ItemNotFoundError` exception. link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION'), + 'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR'), 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -201,7 +201,7 @@ def test_fail_data_with_bad_location(self): }]) }) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") def test_fail_for_non_video_module(self): # non_video module: setup @@ -211,7 +211,7 @@ def test_fail_for_non_video_module(self): 'type': 'non_video' } resp = self.client.ajax_post('/xblock', data) - item_location = self._get_location(resp) + item_locator, item_location = self._get_locator(resp) data = '' modulestore().update_item(item_location, data) @@ -220,7 +220,7 @@ def test_fail_for_non_video_module(self): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': item_location, + 'locator': item_locator, 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -238,7 +238,7 @@ def test_fail_bad_xml(self): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -255,7 +255,7 @@ def test_fail_bad_data_srt_file(self): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.bad_data_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.bad_data_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -270,7 +270,7 @@ def test_fail_bad_name_srt_file(self): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.bad_name_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.bad_name_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -297,7 +297,7 @@ def test_undefined_file_extension(self): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -359,7 +359,7 @@ def test_success_download_youtube(self): self.save_subs_to_store(subs, 'JMD_ifUUfsU') link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location, 'subs_id': "JMD_ifUUfsU"}) + resp = self.client.get(link, {'locator': self.item_locator, 'subs_id': "JMD_ifUUfsU"}) self.assertEqual(resp.status_code, 200) self.assertEqual(resp.content, """0\n00:00:00,100 --> 00:00:00,200\nsubs #1\n\n1\n00:00:00,200 --> 00:00:00,240\nsubs #2\n\n2\n00:00:00,240 --> 00:00:00,380\nsubs #3\n\n""") @@ -386,7 +386,7 @@ def test_success_download_nonyoutube(self): self.save_subs_to_store(subs, subs_id) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location, 'subs_id': subs_id}) + resp = self.client.get(link, {'locator': self.item_locator, 'subs_id': subs_id}) self.assertEqual(resp.status_code, 200) self.assertEqual( resp.content, @@ -397,21 +397,21 @@ def test_success_download_nonyoutube(self): def test_fail_data_without_file(self): link = reverse('download_transcripts') - resp = self.client.get(link, {'id': ''}) + resp = self.client.get(link, {'locator': ''}) self.assertEqual(resp.status_code, 404) resp = self.client.get(link, {}) self.assertEqual(resp.status_code, 404) - def test_fail_data_with_bad_location(self): + def test_fail_data_with_bad_locator(self): # Test for raising `InvalidLocationError` exception. link = reverse('download_transcripts') - resp = self.client.get(link, {'id': 'BAD_LOCATION'}) + resp = self.client.get(link, {'locator': 'BAD_LOCATOR'}) self.assertEqual(resp.status_code, 404) # Test for raising `ItemNotFoundError` exception. link = reverse('download_transcripts') - resp = self.client.get(link, {'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION')}) + resp = self.client.get(link, {'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR')}) self.assertEqual(resp.status_code, 404) def test_fail_for_non_video_module(self): @@ -422,7 +422,7 @@ def test_fail_for_non_video_module(self): 'type': 'videoalpha' } resp = self.client.ajax_post('/xblock', data) - item_location = self._get_location(resp) + item_locator, item_location = self._get_locator(resp) subs_id = str(uuid4()) data = textwrap.dedent(""" @@ -445,7 +445,7 @@ def test_fail_for_non_video_module(self): self.save_subs_to_store(subs, subs_id) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': item_location}) + resp = self.client.get(link, {'locator': item_locator}) self.assertEqual(resp.status_code, 404) def test_fail_nonyoutube_subs_dont_exist(self): @@ -459,7 +459,7 @@ def test_fail_nonyoutube_subs_dont_exist(self): modulestore().update_item(self.item_location, data) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location}) + resp = self.client.get(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 404) def test_empty_youtube_attr_and_sub_attr(self): @@ -473,7 +473,7 @@ def test_empty_youtube_attr_and_sub_attr(self): modulestore().update_item(self.item_location, data) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location}) + resp = self.client.get(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 404) @@ -498,7 +498,7 @@ def test_fail_bad_sjson_subs(self): self.save_subs_to_store(subs, 'JMD_ifUUfsU') link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location}) + resp = self.client.get(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 404) @@ -553,7 +553,7 @@ def test_success_download_nonyoutube(self): self.save_subs_to_store(subs, subs_id) data = { - 'id': self.item_location, + 'locator': self.item_locator, 'videos': [{ 'type': 'html5', 'video': subs_id, @@ -597,7 +597,7 @@ def test_check_youtube(self): self.save_subs_to_store(subs, 'JMD_ifUUfsU') link = reverse('check_transcripts') data = { - 'id': self.item_location, + 'locator': self.item_locator, 'videos': [{ 'type': 'youtube', 'video': 'JMD_ifUUfsU', @@ -625,7 +625,7 @@ def test_check_youtube(self): def test_fail_data_without_id(self): link = reverse('check_transcripts') data = { - 'id': '', + 'locator': '', 'videos': [{ 'type': '', 'video': '', @@ -634,13 +634,13 @@ def test_fail_data_without_id(self): } resp = self.client.get(link, {'data': json.dumps(data)}) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") - def test_fail_data_with_bad_location(self): + def test_fail_data_with_bad_locator(self): # Test for raising `InvalidLocationError` exception. link = reverse('check_transcripts') data = { - 'id': '', + 'locator': '', 'videos': [{ 'type': '', 'video': '', @@ -649,11 +649,11 @@ def test_fail_data_with_bad_location(self): } resp = self.client.get(link, {'data': json.dumps(data)}) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") # Test for raising `ItemNotFoundError` exception. data = { - 'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION'), + 'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR'), 'videos': [{ 'type': '', 'video': '', @@ -662,7 +662,7 @@ def test_fail_data_with_bad_location(self): } resp = self.client.get(link, {'data': json.dumps(data)}) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") def test_fail_for_non_video_module(self): # Not video module: setup @@ -672,7 +672,7 @@ def test_fail_for_non_video_module(self): 'type': 'not_video' } resp = self.client.ajax_post('/xblock', data) - item_location = self._get_location(resp) + item_locator, item_location = self._get_locator(resp) subs_id = str(uuid4()) data = textwrap.dedent(""" @@ -695,7 +695,7 @@ def test_fail_for_non_video_module(self): self.save_subs_to_store(subs, subs_id) data = { - 'id': item_location, + 'locator': item_locator, 'videos': [{ 'type': '', 'video': '', diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 3742c7af203e..ce20e004ee8a 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -223,13 +223,9 @@ def unit_handler(request, tag=None, course_id=None, branch=None, version_guid=No ) components = [ - [ - # TODO: old location needed for video transcripts. - component.location.url(), - loc_mapper().translate_location( - course.location.course_id, component.location, False, True - ) - ] + loc_mapper().translate_location( + course.location.course_id, component.location, False, True + ) for component in item.get_children() ] diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 9db2cb9c1305..9d5cdf8e80f7 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -18,11 +18,12 @@ from xmodule.contentstore.content import StaticContent from xmodule.exceptions import NotFoundError -from xmodule.modulestore.django import modulestore +from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.contentstore.django import contentstore -from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError +from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError from util.json_request import JsonResponse +from xmodule.modulestore.locator import BlockUsageLocator from ..transcripts_utils import ( generate_subs_from_source, @@ -77,20 +78,14 @@ def upload_transcripts(request): 'subs': '', } - item_location = request.POST.get('id') - if not item_location: - return error_response(response, 'POST data without "id" form data.') + locator = request.POST.get('locator') + if not locator: + return error_response(response, 'POST data without "locator" form data.') - # This is placed before has_access() to validate item_location, - # because has_access() raises InvalidLocationError if location is invalid. try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - return error_response(response, "Can't find item by location.") - - # Check permissions for this user within this course. - if not has_access(request.user, item_location): - raise PermissionDenied() + item = _get_item(request, request.POST) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + return error_response(response, "Can't find item by locator.") if 'file' not in request.FILES: return error_response(response, 'POST data without "file" form data.') @@ -156,23 +151,17 @@ def download_transcripts(request): Raises Http404 if unsuccessful. """ - item_location = request.GET.get('id') - if not item_location: - log.debug('GET data without "id" property.') + locator = request.GET.get('locator') + if not locator: + log.debug('GET data without "locator" property.') raise Http404 - # This is placed before has_access() to validate item_location, - # because has_access() raises InvalidLocationError if location is invalid. try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - log.debug("Can't find item by location.") + item = _get_item(request, request.GET) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + log.debug("Can't find item by locator.") raise Http404 - # Check permissions for this user within this course. - if not has_access(request.user, item_location): - raise PermissionDenied() - subs_id = request.GET.get('subs_id') if not subs_id: log.debug('GET data without "subs_id" property.') @@ -240,7 +229,7 @@ def check_transcripts(request): 'status': 'Error', } try: - __, videos, item = validate_transcripts_data(request) + __, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(transcripts_presence, e.message) @@ -303,7 +292,7 @@ def check_transcripts(request): if len(html5_subs) == 2: # check html5 transcripts for equality transcripts_presence['html5_equal'] = json.loads(html5_subs[0]) == json.loads(html5_subs[1]) - command, subs_to_use = transcripts_logic(transcripts_presence, videos) + command, subs_to_use = _transcripts_logic(transcripts_presence, videos) transcripts_presence.update({ 'command': command, 'subs': subs_to_use, @@ -311,7 +300,7 @@ def check_transcripts(request): return JsonResponse(transcripts_presence) -def transcripts_logic(transcripts_presence, videos): +def _transcripts_logic(transcripts_presence, videos): """ By `transcripts_presence` content, figure what show to user: @@ -386,7 +375,7 @@ def choose_transcripts(request): } try: - data, videos, item = validate_transcripts_data(request) + data, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(response, e.message) @@ -416,7 +405,7 @@ def replace_transcripts(request): response = {'status': 'Error', 'subs': ''} try: - __, videos, item = validate_transcripts_data(request) + __, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(response, e.message) @@ -435,7 +424,7 @@ def replace_transcripts(request): return JsonResponse(response) -def validate_transcripts_data(request): +def _validate_transcripts_data(request): """ Validates, that request contains all proper data for transcripts processing. @@ -452,18 +441,10 @@ def validate_transcripts_data(request): if not data: raise TranscriptsRequestValidationException('Incoming video data is empty.') - item_location = data.get('id') - - # This is placed before has_access() to validate item_location, - # because has_access() raises InvalidLocationError if location is invalid. try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - raise TranscriptsRequestValidationException("Can't find item by location.") - - # Check permissions for this user within this course. - if not has_access(request.user, item_location): - raise PermissionDenied() + item = _get_item(request, data) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + raise TranscriptsRequestValidationException("Can't find item by locator.") if item.category != 'video': raise TranscriptsRequestValidationException('Transcripts are supported only for "video" modules.') @@ -492,7 +473,7 @@ def rename_transcripts(request): response = {'status': 'Error', 'subs': ''} try: - __, videos, item = validate_transcripts_data(request) + __, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(response, e.message) @@ -525,11 +506,10 @@ def save_transcripts(request): if not data: return error_response(response, 'Incoming video data is empty.') - item_location = data.get('id') try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - return error_response(response, "Can't find item by location.") + item = _get_item(request, data) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + return error_response(response, "Can't find item by locator.") metadata = data.get('metadata') if metadata is not None: @@ -553,3 +533,24 @@ def save_transcripts(request): response['status'] = 'Success' return JsonResponse(response) + + +def _get_item(request, data): + """ + Obtains from 'data' the locator for an item. + Next, gets that item from the modulestore (allowing any errors to raise up). + Finally, verifies that the user has access to the item. + + Returns the item. + """ + locator = BlockUsageLocator(data.get('locator')) + old_location = loc_mapper().translate_locator_to_location(locator) + + # This is placed before has_access() to validate the location, + # because has_access() raises InvalidLocationError if location is invalid. + item = modulestore().get_item(old_location) + + if not has_access(request.user, locator): + raise PermissionDenied() + + return item diff --git a/cms/static/js/spec/transcripts/file_uploader_spec.js b/cms/static/js/spec/transcripts/file_uploader_spec.js index c896ad811ae2..20b653527cf6 100644 --- a/cms/static/js/spec/transcripts/file_uploader_spec.js +++ b/cms/static/js/spec/transcripts/file_uploader_spec.js @@ -48,7 +48,7 @@ function ($, _, Utils, FileUploader) { el: $container, messenger: messenger, videoListObject: videoListObject, - component_id: 'component_id' + component_locator: 'component_locator' }); }); diff --git a/cms/static/js/spec/transcripts/message_manager_spec.js b/cms/static/js/spec/transcripts/message_manager_spec.js index c6d3bc790967..40eef7f02e00 100644 --- a/cms/static/js/spec/transcripts/message_manager_spec.js +++ b/cms/static/js/spec/transcripts/message_manager_spec.js @@ -52,7 +52,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { view = new MessageManager({ el: $container, parent: videoList, - component_id: 'component_id' + component_locator: 'component_locator' }); }); @@ -60,7 +60,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { expect(fileUploader.initialize).toHaveBeenCalledWith({ el: view.$el, messenger: view, - component_id: view.component_id, + component_locator: view.component_locator, videoListObject: view.options.parent }); }); @@ -215,7 +215,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { function() { expect(Utils.command).toHaveBeenCalledWith( action, - view.component_id, + view.component_locator, videoList, void(0) ); @@ -245,7 +245,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { function () { expect(Utils.command).toHaveBeenCalledWith( action, - view.component_id, + view.component_locator, videoList, { html5_id: extraParamas @@ -268,7 +268,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { function () { expect(Utils.command).toHaveBeenCalledWith( action, - view.component_id, + view.component_locator, videoList, void(0) ); diff --git a/cms/static/js/spec/transcripts/videolist_spec.js b/cms/static/js/spec/transcripts/videolist_spec.js index cf189c254978..de850eb3eed3 100644 --- a/cms/static/js/spec/transcripts/videolist_spec.js +++ b/cms/static/js/spec/transcripts/videolist_spec.js @@ -11,7 +11,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s 'transcripts/metadata-videolist-entry.underscore' ), abstractEditor = AbstractEditor.prototype, - component_id = 'component_id', + component_locator = 'component_locator', videoList = [ { mode: "youtube", @@ -62,7 +62,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s var tpl = sandbox({ 'class': 'component', - 'data-id': component_id + 'data-locator': component_locator }), model = new MetadataModel(modelStub), videoList, $el; @@ -157,7 +157,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s waitsForResponse(function () { expect(abstractEditor.initialize).toHaveBeenCalled(); expect(messenger.initialize).toHaveBeenCalled(); - expect(view.component_id).toBe(component_id); + expect(view.component_locator).toBe(component_locator); expect(view.$el).toHandle('input'); }); }); @@ -167,7 +167,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s expect(abstractEditor.render).toHaveBeenCalled(); expect(Utils.command).toHaveBeenCalledWith( 'check', - component_id, + component_locator, videoList ); diff --git a/cms/static/js/views/transcripts/editor.js b/cms/static/js/views/transcripts/editor.js index 21e48a03b04b..78280a0d66c9 100644 --- a/cms/static/js/views/transcripts/editor.js +++ b/cms/static/js/views/transcripts/editor.js @@ -72,7 +72,7 @@ function($, Backbone, _, Utils, MetadataView, MetadataCollection) { syncBasicTab: function (metadataCollection, metadataView) { var result = [], getField = Utils.getField, - component_id = this.$el.closest('.component').data('id'), + component_locator = this.$el.closest('.component').data('locator'), subs = getField(metadataCollection, 'sub'), values = {}, videoUrl, metadata, modifiedValues; @@ -99,7 +99,7 @@ function($, Backbone, _, Utils, MetadataView, MetadataCollection) { if (isSubsModified) { metadata = $.extend(true, {}, modifiedValues); // Save module state - Utils.command('save', component_id, null, { + Utils.command('save', component_locator, null, { metadata: metadata, current_subs: _.pluck( Utils.getVideoList(videoUrl.getDisplayValue()), @@ -110,18 +110,16 @@ function($, Backbone, _, Utils, MetadataView, MetadataCollection) { // Get values from `Advanced` tab fields (`html5_sources`, // `youtube_id_1_0`) that should be synchronized. - html5Sources = getField(metadataCollection, 'html5_sources') -                                    .getDisplayValue(); + var html5Sources = getField(metadataCollection, 'html5_sources').getDisplayValue(); -            values.youtube = getField(metadataCollection, 'youtube_id_1_0') -                                    .getDisplayValue(); + values.youtube = getField(metadataCollection, 'youtube_id_1_0').getDisplayValue(); -            values.html5Sources = _.filter(html5Sources, function (value) { -                var link = Utils.parseLink(value), + values.html5Sources = _.filter(html5Sources, function (value) { + var link = Utils.parseLink(value), mode = link && link.mode; -                return mode === 'html5' && mode; -            }); + return mode === 'html5' && mode; + }); // The length of youtube video_id should be 11 characters. diff --git a/cms/static/js/views/transcripts/file_uploader.js b/cms/static/js/views/transcripts/file_uploader.js index c9c33820e43b..d307c7c85e39 100644 --- a/cms/static/js/views/transcripts/file_uploader.js +++ b/cms/static/js/views/transcripts/file_uploader.js @@ -39,7 +39,7 @@ function($, Backbone, _, Utils) { tplContainer.html(this.template({ ext: this.validFileExtensions, - component_id: this.options.component_id, + component_locator: this.options.component_locator, video_list: videoList })); diff --git a/cms/static/js/views/transcripts/message_manager.js b/cms/static/js/views/transcripts/message_manager.js index 80f70752cfee..506f3143ac2b 100644 --- a/cms/static/js/views/transcripts/message_manager.js +++ b/cms/static/js/views/transcripts/message_manager.js @@ -31,12 +31,12 @@ function($, Backbone, _, Utils, FileUploader, gettext) { initialize: function () { _.bindAll(this); - this.component_id = this.$el.closest('.component').data('id'); + this.component_locator = this.$el.closest('.component').data('locator'); this.fileUploader = new FileUploader({ el: this.$el, messenger: this, - component_id: this.component_id, + component_locator: this.component_locator, videoListObject: this.options.parent }); }, @@ -76,7 +76,7 @@ function($, Backbone, _, Utils, FileUploader, gettext) { this.$el.find('.transcripts-status') .removeClass('is-invisible') .find(this.elClass).html(template({ - component_id: encodeURIComponent(this.component_id), + component_locator: encodeURIComponent(this.component_locator), html5_list: html5List, grouped_list: groupedList, subs_id: (params) ? params.subs: '' @@ -204,7 +204,7 @@ function($, Backbone, _, Utils, FileUploader, gettext) { */ processCommand: function (action, errorMessage, videoId) { var self = this, - component_id = this.component_id, + component_locator = this.component_locator, videoList = this.options.parent.getVideoObjectsList(), extraParam, xhr; @@ -212,7 +212,7 @@ function($, Backbone, _, Utils, FileUploader, gettext) { extraParam = { html5_id: videoId }; } - xhr = Utils.command(action, component_id, videoList, extraParam) + xhr = Utils.command(action, component_locator, videoList, extraParam) .done(function (resp) { var sub = resp.subs; diff --git a/cms/static/js/views/transcripts/metadata_videolist.js b/cms/static/js/views/transcripts/metadata_videolist.js index 6e7e82232b10..7e7dac7f1956 100644 --- a/cms/static/js/views/transcripts/metadata_videolist.js +++ b/cms/static/js/views/transcripts/metadata_videolist.js @@ -46,7 +46,7 @@ function($, Backbone, _, AbstractEditor, Utils, MessageManager, MetadataView) { _.debounce(_.bind(this.inputHandler, this), this.inputDelay) ); - this.component_id = this.$el.closest('.component').data('id'); + this.component_locator = this.$el.closest('.component').data('locator'); }, render: function () { @@ -55,7 +55,7 @@ function($, Backbone, _, AbstractEditor, Utils, MessageManager, MetadataView) { .apply(this, arguments); var self = this, - component_id = this.$el.closest('.component').data('id'), + component_locator = this.$el.closest('.component').data('locator'), videoList = this.getVideoObjectsList(), showServerError = function (response) { @@ -82,7 +82,7 @@ function($, Backbone, _, AbstractEditor, Utils, MessageManager, MetadataView) { } // Check current state of Timed Transcripts. - Utils.command('check', component_id, videoList) + Utils.command('check', component_locator, videoList) .done(function (resp) { var params = resp, len = videoList.length, diff --git a/cms/static/js/views/transcripts/utils.js b/cms/static/js/views/transcripts/utils.js index 7965d0b9e306..a6865906e57b 100644 --- a/cms/static/js/views/transcripts/utils.js +++ b/cms/static/js/views/transcripts/utils.js @@ -295,7 +295,7 @@ define(["jquery", "underscore", "jquery.ajaxQueue"], function($, _) { * * @param {string} action Action that will be invoked on server. Is a part * of url. - * @param {string} component_id Id of component. + * @param {string} component_locator the locator of component. * @param {array} videoList List of object with information about inserted * urls. * @param {object} extraParams Extra parameters that can be send to the @@ -314,7 +314,7 @@ define(["jquery", "underscore", "jquery.ajaxQueue"], function($, _) { // _command() function. var xhr = null; - return function (action, component_id, videoList, extraParams) { + return function (action, component_locator, videoList, extraParams) { var params, data; if (extraParams) { @@ -326,7 +326,7 @@ define(["jquery", "underscore", "jquery.ajaxQueue"], function($, _) { } data = $.extend( - { id: component_id }, + { locator: component_locator }, { videos: videoList }, params ); diff --git a/cms/templates/js/transcripts/file-upload.underscore b/cms/templates/js/transcripts/file-upload.underscore index 90dfb80db663..d48f9af8873e 100644 --- a/cms/templates/js/transcripts/file-upload.underscore +++ b/cms/templates/js/transcripts/file-upload.underscore @@ -5,6 +5,6 @@ method="post" enctype="multipart/form-data"> - + diff --git a/cms/templates/js/transcripts/messages/transcripts-found.underscore b/cms/templates/js/transcripts/messages/transcripts-found.underscore index 6a12edcd3fb7..286063ed6a33 100644 --- a/cms/templates/js/transcripts/messages/transcripts-found.underscore +++ b/cms/templates/js/transcripts/messages/transcripts-found.underscore @@ -10,7 +10,7 @@ - "> + "> <%= gettext("Download to Edit") %> diff --git a/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore b/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore index fa328208fde6..c17d83f71078 100644 --- a/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore +++ b/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore @@ -10,7 +10,7 @@ - "> + "> <%= gettext("Download to Edit") %> diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 9f3daf0b20c8..2318e9cd9725 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -48,8 +48,8 @@

    - % for id, locator in components: -
  1. + % for locator in components: +
  2. % endfor
  3. From 050031f7d5bc04389a5018f75c92b7b25dbfec83 Mon Sep 17 00:00:00 2001 From: Zubair Afzal Date: Wed, 4 Dec 2013 16:43:17 +0500 Subject: [PATCH 2/5] Disable Peer Track Changes STUD-1008 --- common/lib/xmodule/xmodule/combined_open_ended_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 68a0d65617c1..2eb8585d854e 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -508,5 +508,5 @@ def get_context(self): def non_editable_metadata_fields(self): non_editable_fields = super(CombinedOpenEndedDescriptor, self).non_editable_metadata_fields non_editable_fields.extend([CombinedOpenEndedDescriptor.due, CombinedOpenEndedDescriptor.graceperiod, - CombinedOpenEndedDescriptor.markdown, CombinedOpenEndedDescriptor.version]) + CombinedOpenEndedDescriptor.markdown, CombinedOpenEndedDescriptor.version, CombinedOpenEndedDescriptor.track_changes]) return non_editable_fields From 76c8a7a38dd87b771583d902730a931ac819815b Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Thu, 5 Dec 2013 14:50:05 -0500 Subject: [PATCH 3/5] add all clear message --- lms/templates/instructor/staff_grading.html | 11 +++++++++++ .../open_ended_problems/combined_notifications.html | 11 +++++++++++ .../open_ended_flagged_problems.html | 11 +++++++++++ .../open_ended_problems/open_ended_problems.html | 11 +++++++++++ lms/templates/peer_grading/peer_grading.html | 11 +++++++++++ 5 files changed, 55 insertions(+) diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index 59585308c182..ae6485ec364f 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -22,6 +22,17 @@

    ${_("Staff grading")}

    +
    +
    + + The issue we had between Dec 3 and Dec 5 with the visibility + of student responses has now been resolved. All responses + should now be visible. Thank you for your patience! + --the edX team + +
    +
    +
    diff --git a/lms/templates/open_ended_problems/combined_notifications.html b/lms/templates/open_ended_problems/combined_notifications.html index 47bb01fc8ed5..1f20256479df 100644 --- a/lms/templates/open_ended_problems/combined_notifications.html +++ b/lms/templates/open_ended_problems/combined_notifications.html @@ -16,6 +16,17 @@
    ${error_text}
    +
    +
    + + The issue we had between Dec 3 and Dec 5 with the visibility + of student responses has now been resolved. All responses + should now be visible. Thank you for your patience! + --the edX team + +
    +
    +

    ${_("Open Ended Console")}

    ${_("Instructions")}

    diff --git a/lms/templates/open_ended_problems/open_ended_flagged_problems.html b/lms/templates/open_ended_problems/open_ended_flagged_problems.html index 87d5a11bd806..3bad135c2876 100644 --- a/lms/templates/open_ended_problems/open_ended_flagged_problems.html +++ b/lms/templates/open_ended_problems/open_ended_flagged_problems.html @@ -19,6 +19,17 @@
    ${error_text}
    +
    +
    + + The issue we had between Dec 3 and Dec 5 with the visibility + of student responses has now been resolved. All responses + should now be visible. Thank you for your patience! + --the edX team + +
    +
    +

    ${_("Flagged Open Ended Problems")}

    ${_("Instructions")}

    diff --git a/lms/templates/open_ended_problems/open_ended_problems.html b/lms/templates/open_ended_problems/open_ended_problems.html index d5edc8edaf6b..f9447f49a9a4 100644 --- a/lms/templates/open_ended_problems/open_ended_problems.html +++ b/lms/templates/open_ended_problems/open_ended_problems.html @@ -16,6 +16,17 @@
    ${error_text}
    +
    +
    + + The issue we had between Dec 3 and Dec 5 with the visibility + of student responses has now been resolved. All responses + should now be visible. Thank you for your patience! + --the edX team + +
    +
    +

    ${_("Open Ended Problems")}

    ${_("Instructions")}

    ${_("Here is a list of open ended problems for this course.")}

    diff --git a/lms/templates/peer_grading/peer_grading.html b/lms/templates/peer_grading/peer_grading.html index 101e6341f527..1b212bcc0f43 100644 --- a/lms/templates/peer_grading/peer_grading.html +++ b/lms/templates/peer_grading/peer_grading.html @@ -15,6 +15,17 @@
    ${error_text}
    +
    +
    + + The issue we had between Dec 3 and Dec 5 with the visibility + of student responses has now been resolved. All responses + should now be visible. Thank you for your patience! + --the edX team + +
    +
    +

    ${_("Peer Grading")}

    ${_("Instructions")}

    From 7e4820af520bf4f83c24ad3c6d72a70c3d56a755 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 9 Dec 2013 16:34:07 -0500 Subject: [PATCH 4/5] Reduce sql queries for groupname tests. (STUD-1039) Conflicts: cms/djangoapps/auth/authz.py --- cms/djangoapps/auth/authz.py | 15 ++++++--------- lms/djangoapps/courseware/roles.py | 1 - 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 1a1f138cb588..5ec7beba2ef4 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -63,7 +63,7 @@ def get_all_course_role_groupnames(location, role, use_filter=True): # filter to the ones which exist default = groupnames[0] if use_filter: - groupnames = [group for group in groupnames if Group.objects.filter(name=group).exists()] + groupnames = [group.name for group in Group.objects.filter(name__in=groupnames)] return groupnames, default @@ -203,12 +203,9 @@ def remove_user_from_course_group(caller, user, location, role): # see if the user is actually in that role, if not then we don't have to do anything groupnames, _ = get_all_course_role_groupnames(location, role) - for groupname in groupnames: - groups = user.groups.filter(name=groupname) - if groups: - # will only be one with that name - user.groups.remove(groups[0]) - user.save() + for group in user.groups.filter(name__in=groupnames): + user.groups.remove(group) + user.save() def remove_user_from_creator_group(caller, user): @@ -243,7 +240,7 @@ def is_user_in_course_group_role(user, location, role, check_staff=True): if check_staff and user.is_staff: return True groupnames, _ = get_all_course_role_groupnames(location, role) - return any(user.groups.filter(name=groupname).exists() for groupname in groupnames) + return user.groups.filter(name__in=groupnames).exists() return False @@ -266,7 +263,7 @@ def is_user_in_creator_group(user): # Feature flag for using the creator group setting. Will be removed once the feature is complete. if settings.MITX_FEATURES.get('ENABLE_CREATOR_GROUP', False): - return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).count() > 0 + return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).exists() return True diff --git a/lms/djangoapps/courseware/roles.py b/lms/djangoapps/courseware/roles.py index 110bb9f3623b..60853e8090d0 100644 --- a/lms/djangoapps/courseware/roles.py +++ b/lms/djangoapps/courseware/roles.py @@ -4,7 +4,6 @@ """ from abc import ABCMeta, abstractmethod -from functools import partial from django.contrib.auth.models import User, Group From aba25c2de7dbb8ec60c276bacf9dcd8019da7c26 Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Mon, 25 Nov 2013 15:35:51 +0000 Subject: [PATCH 5/5] In OpenEndedModule latest_score and all_score calculate sum of rubric scores every time for ML grading. ORA-72 --- .../combined_open_ended_modulev1.py | 19 +- .../open_ended_module.py | 40 +- .../xmodule/tests/test_combined_open_ended.py | 182 ++++- .../xmodule/tests/test_util_open_ended.py | 670 ++++++++++++++++++ 4 files changed, 903 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 72915eb7b36f..3158ff291ecd 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -258,8 +258,23 @@ def states_sort_key(self, idx_task_states): if not task_states: return (0, 0, state_values[OpenEndedChild.INITIAL], idx) - final_child_state = json.loads(task_states[-1]) - scores = [attempt.get('score', 0) for attempt in final_child_state.get('child_history', [])] + final_task_xml = self.task_xml[-1] + final_child_state_json = task_states[-1] + final_child_state = json.loads(final_child_state_json) + + tag_name = self.get_tag_name(final_task_xml) + children = self.child_modules() + task_descriptor = children['descriptors'][tag_name](self.system) + task_parsed_xml = task_descriptor.definition_from_xml(etree.fromstring(final_task_xml), self.system) + task = children['modules'][tag_name]( + self.system, + self.location, + task_parsed_xml, + task_descriptor, + self.static_data, + instance_state=final_child_state_json, + ) + scores = task.all_scores() if scores: best_score = max(scores) else: diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index d020987fdf6c..35c1c361ccbc 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -679,7 +679,7 @@ def save_answer(self, data, system): return { 'success': success, 'error': error_message, - 'student_response': data['student_answer'].replace("\n","
    ") + 'student_response': data['student_answer'].replace("\n", "
    ") } def update_score(self, data, system): @@ -738,6 +738,44 @@ def get_html(self, system): html = system.render_template('{0}/open_ended.html'.format(self.TEMPLATE_DIR), context) return html + def latest_score(self): + """None if not available""" + if not self.child_history: + return None + return self.score_for_attempt(-1) + + def all_scores(self): + """None if not available""" + if not self.child_history: + return None + return [self.score_for_attempt(index) for index in xrange(0, len(self.child_history))] + + def score_for_attempt(self, index): + """ + Return sum of rubric scores for ML grading otherwise return attempt["score"]. + """ + attempt = self.child_history[index] + score = attempt.get('score') + post_assessment_data = self._parse_score_msg(attempt.get('post_assessment'), self.system) + grader_types = post_assessment_data.get('grader_types') + + # According to _parse_score_msg in ML grading there should be only one grader type. + if len(grader_types) == 1 and grader_types[0] == 'ML': + rubric_scores = post_assessment_data.get("rubric_scores") + + # Similarly there should be only one list of rubric scores. + if len(rubric_scores) == 1: + rubric_scores_sum = sum(rubric_scores[0]) + log.debug("""Score normalized for location={loc}, old_score={old_score}, + new_score={new_score}, rubric_score={rubric_score}""".format( + loc=self.location_string, + old_score=score, + new_score=rubric_scores_sum, + rubric_score=rubric_scores + )) + return rubric_scores_sum + return score + class OpenEndedDescriptor(): """ diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index e1b2a4ebe241..6498c82c4ab3 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -27,7 +27,9 @@ from xmodule.tests.test_util_open_ended import ( DummyModulestore, TEST_STATE_SA_IN, MOCK_INSTANCE_STATE, TEST_STATE_SA, TEST_STATE_AI, TEST_STATE_AI2, TEST_STATE_AI2_INVALID, - TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE, MockUploadedFile + TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE, MockUploadedFile, INSTANCE_INCONSISTENT_STATE, + INSTANCE_INCONSISTENT_STATE2, INSTANCE_INCONSISTENT_STATE3, INSTANCE_INCONSISTENT_STATE4, + INSTANCE_INCONSISTENT_STATE5 ) from xblock.field_data import DictFieldData @@ -358,7 +360,7 @@ def test_open_ended_display(self): # Create a module with no state yet. Important that this start off as a blank slate. test_module = OpenEndedModule(self.test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) + self.definition, self.descriptor, self.static_data, self.metadata) saved_response = "Saved response." submitted_response = "Submitted response." @@ -369,7 +371,7 @@ def test_open_ended_display(self): self.assertEqual(test_module.get_display_answer(), "") # Now, store an answer in the module. - test_module.handle_ajax("store_answer", {'student_answer' : saved_response}, get_test_system()) + test_module.handle_ajax("store_answer", {'student_answer': saved_response}, get_test_system()) # The stored answer should now equal our response. self.assertEqual(test_module.stored_answer, saved_response) self.assertEqual(test_module.get_display_answer(), saved_response) @@ -387,6 +389,7 @@ def test_open_ended_display(self): # Confirm that the answer is stored properly. self.assertEqual(test_module.latest_answer(), submitted_response) + class CombinedOpenEndedModuleTest(unittest.TestCase): """ Unit tests for the combined open ended xmodule @@ -610,7 +613,6 @@ def test_alternate_orderings(self): metadata=self.metadata, instance_state={'task_states': TEST_STATE_SA_IN}) - def test_get_score_realistic(self): """ Try to parse the correct score from a json instance state @@ -717,6 +719,175 @@ def test_state_pe_single(self): self.ai_state_success(TEST_STATE_PE_SINGLE, iscore=0, tasks=[self.task_xml2]) +class CombinedOpenEndedModuleConsistencyTest(unittest.TestCase): + """ + Unit tests for the combined open ended xmodule rubric scores consistency. + """ + + # location, definition_template, prompt, rubric, max_score, metadata, oeparam, task_xml1, task_xml2 + # All these variables are used to construct the xmodule descriptor. + location = Location(["i4x", "edX", "open_ended", "combinedopenended", + "SampleQuestion"]) + definition_template = """ + + {rubric} + {prompt} + + {task1} + + + {task2} + + + """ + prompt = "This is a question prompt" + rubric = ''' + + Response Quality + + + + ''' + max_score = 10 + + metadata = {'attempts': '10', 'max_score': max_score} + + oeparam = etree.XML(''' + + Enter essay here. + This is the answer. + {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} + + ''') + + task_xml1 = ''' + + + What hint about this problem would you give to someone? + + + Save Succcesful. Thanks for participating! + + + ''' + task_xml2 = ''' + + + Enter essay here. + This is the answer. + {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} + + ''' + + + static_data = { + 'max_attempts': 20, + 'prompt': prompt, + 'rubric': rubric, + 'max_score': max_score, + 'display_name': 'Name', + 'accept_file_upload': False, + 'close_date': "", + 's3_interface': test_util_open_ended.S3_INTERFACE, + 'open_ended_grading_interface': test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE, + 'skip_basic_checks': False, + 'graded': True, + } + + definition = {'prompt': etree.XML(prompt), 'rubric': etree.XML(rubric), 'task_xml': [task_xml1, task_xml2]} + full_definition = definition_template.format(prompt=prompt, rubric=rubric, task1=task_xml1, task2=task_xml2) + descriptor = Mock(data=full_definition) + test_system = get_test_system() + test_system.open_ended_grading_interface = None + combinedoe_container = CombinedOpenEndedModule( + descriptor=descriptor, + runtime=test_system, + field_data=DictFieldData({ + 'data': full_definition, + 'weight': '1', + }), + scope_ids=ScopeIds(None, None, None, None), + ) + + def setUp(self): + self.combinedoe = CombinedOpenEndedV1Module(self.test_system, + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=json.loads(INSTANCE_INCONSISTENT_STATE)) + + def test_get_score(self): + """ + If grader type is ML score should be updated from rubric scores. Aggregate rubric scores = sum([3])*5. + """ + score_dict = self.combinedoe.get_score() + self.assertEqual(score_dict['score'], 15.0) + self.assertEqual(score_dict['total'], 5.0) + + def test_get_score_with_pe_grader(self): + """ + If grader type is PE score should not be updated from rubric scores. Aggregate rubric scores = sum([3])*5. + """ + combinedoe = CombinedOpenEndedV1Module(self.test_system, + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=json.loads(INSTANCE_INCONSISTENT_STATE2)) + score_dict = combinedoe.get_score() + self.assertNotEqual(score_dict['score'], 15.0) + + def test_get_score_with_different_score_value_in_rubric(self): + """ + If grader type is ML score should be updated from rubric scores. Aggregate rubric scores = sum([5])*5. + """ + combinedoe = CombinedOpenEndedV1Module(self.test_system, + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=json.loads(INSTANCE_INCONSISTENT_STATE3)) + score_dict = combinedoe.get_score() + self.assertEqual(score_dict['score'], 25.0) + self.assertEqual(score_dict['total'], 5.0) + + def test_get_score_with_old_task_states(self): + """ + If grader type is ML and old_task_states are present in instance inconsistent state score should be updated + from rubric scores. Aggregate rubric scores = sum([3])*5. + """ + combinedoe = CombinedOpenEndedV1Module(self.test_system, + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=json.loads(INSTANCE_INCONSISTENT_STATE4)) + score_dict = combinedoe.get_score() + self.assertEqual(score_dict['score'], 15.0) + self.assertEqual(score_dict['total'], 5.0) + + def test_get_score_with_score_missing(self): + """ + If grader type is ML and score field is missing in instance inconsistent state score should be updated from + rubric scores. Aggregate rubric scores = sum([3])*5. + """ + combinedoe = CombinedOpenEndedV1Module(self.test_system, + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=json.loads(INSTANCE_INCONSISTENT_STATE5)) + score_dict = combinedoe.get_score() + self.assertEqual(score_dict['score'], 15.0) + self.assertEqual(score_dict['total'], 5.0) + + class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): """ Test the student flow in the combined open ended xmodule @@ -948,6 +1119,7 @@ def test_reset_fail(self): reset_data = json.loads(self._handle_ajax("reset", {})) self.assertEqual(reset_data['success'], False) + class OpenEndedModuleXmlImageUploadTest(unittest.TestCase, DummyModulestore): """ Test if student is able to upload images properly. @@ -1018,7 +1190,7 @@ def test_link_submission_success(self): # Simulate a student saving an answer with a link. response = module.handle_ajax("save_answer", { "student_answer": "{0} {1}".format(self.answer_text, self.answer_link) - }) + }) response = json.loads(response) diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index bbb0653512e6..f4a607aecf3c 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -1,3 +1,5 @@ +import json +from textwrap import dedent from xmodule.modulestore import Location from xmodule.modulestore.xml import XMLModuleStore from xmodule.tests import DATA_DIR, get_test_system @@ -98,12 +100,680 @@ def get_module_from_location(self, location, course): descriptor.xmodule_runtime = self.get_module_system(descriptor) return descriptor +def serialize_child_history(task_state): + """ + To json serialize feedback and post_assessment in child_history of task state. + """ + child_history = task_state.get("child_history", []) + for i, attempt in enumerate(child_history): + if "post_assessment" in attempt: + if "feedback" in attempt["post_assessment"]: + attempt["post_assessment"]["feedback"] = json.dumps(attempt["post_assessment"].get("feedback")) + task_state["child_history"][i]["post_assessment"] = json.dumps(attempt["post_assessment"]) + +def serialize_open_ended_instance_state(json_str): + """ + To json serialize task_states and old_task_states in instance state. + """ + json_data = json.loads(json_str) + task_states = json_data.get('task_states', []) + for i, task_state in enumerate(task_states): + serialize_child_history(task_state) + json_data['task_states'][i] = json.dumps(task_state) + + old_task_states = json_data.get('old_task_states', []) + for i, old_task in enumerate(old_task_states): + for j, task_state in enumerate(old_task): + old_task[j] = json.dumps(task_state) + json_data['old_task_states'][i] = old_task + + return json.dumps(json_data) + + # Task state for a module with self assessment then instructor assessment. TEST_STATE_SA_IN = ["{\"child_created\": false, \"child_attempts\": 2, \"version\": 1, \"child_history\": [{\"answer\": \"However venture pursuit he am mr cordial. Forming musical am hearing studied be luckily. Ourselves for determine attending how led gentleman sincerity. Valley afford uneasy joy she thrown though bed set. In me forming general prudent on country carried. Behaved an or suppose justice. Seemed whence how son rather easily and change missed. Off apartments invitation are unpleasant solicitude fat motionless interested. Hardly suffer wisdom wishes valley as an. As friendship advantages resolution it alteration stimulated he or increasing. \\r

    Now led tedious shy lasting females off. Dashwood marianne in of entrance be on wondered possible building. Wondered sociable he carriage in speedily margaret. Up devonshire of he thoroughly insensible alteration. An mr settling occasion insisted distance ladyship so. Not attention say frankness intention out dashwoods now curiosity. Stronger ecstatic as no judgment daughter speedily thoughts. Worse downs nor might she court did nay forth these. \", \"post_assessment\": \"[3, 3, 2, 2, 2]\", \"score\": 12}, {\"answer\": \"Delightful remarkably mr on announcing themselves entreaties favourable. About to in so terms voice at. Equal an would is found seems of. The particular friendship one sufficient terminated frequently themselves. It more shed went up is roof if loud case. Delay music in lived noise an. Beyond genius really enough passed is up. \\r

    John draw real poor on call my from. May she mrs furnished discourse extremely. Ask doubt noisy shade guest did built her him. Ignorant repeated hastened it do. Consider bachelor he yourself expenses no. Her itself active giving for expect vulgar months. Discovery commanded fat mrs remaining son she principle middleton neglected. Be miss he in post sons held. No tried is defer do money scale rooms. \", \"post_assessment\": \"[3, 3, 2, 2, 2]\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"done\"}", "{\"child_created\": false, \"child_attempts\": 0, \"version\": 1, \"child_history\": [{\"answer\": \"However venture pursuit he am mr cordial. Forming musical am hearing studied be luckily. Ourselves for determine attending how led gentleman sincerity. Valley afford uneasy joy she thrown though bed set. In me forming general prudent on country carried. Behaved an or suppose justice. Seemed whence how son rather easily and change missed. Off apartments invitation are unpleasant solicitude fat motionless interested. Hardly suffer wisdom wishes valley as an. As friendship advantages resolution it alteration stimulated he or increasing. \\r

    Now led tedious shy lasting females off. Dashwood marianne in of entrance be on wondered possible building. Wondered sociable he carriage in speedily margaret. Up devonshire of he thoroughly insensible alteration. An mr settling occasion insisted distance ladyship so. Not attention say frankness intention out dashwoods now curiosity. Stronger ecstatic as no judgment daughter speedily thoughts. Worse downs nor might she court did nay forth these. \", \"post_assessment\": \"{\\\"submission_id\\\": 1460, \\\"score\\\": 12, \\\"feedback\\\": \\\"{\\\\\\\"feedback\\\\\\\": \\\\\\\"\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 5413, \\\"grader_type\\\": \\\"IN\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"\\\\nIdeas\\\\n3\\\\nContent\\\\n3\\\\nOrganization\\\\n2\\\\nStyle\\\\n2\\\\nVoice\\\\n2\\\"}\", \"score\": 12}, {\"answer\": \"Delightful remarkably mr on announcing themselves entreaties favourable. About to in so terms voice at. Equal an would is found seems of. The particular friendship one sufficient terminated frequently themselves. It more shed went up is roof if loud case. Delay music in lived noise an. Beyond genius really enough passed is up. \\r

    John draw real poor on call my from. May she mrs furnished discourse extremely. Ask doubt noisy shade guest did built her him. Ignorant repeated hastened it do. Consider bachelor he yourself expenses no. Her itself active giving for expect vulgar months. Discovery commanded fat mrs remaining son she principle middleton neglected. Be miss he in post sons held. No tried is defer do money scale rooms. \", \"post_assessment\": \"{\\\"submission_id\\\": 1462, \\\"score\\\": 12, \\\"feedback\\\": \\\"{\\\\\\\"feedback\\\\\\\": \\\\\\\"\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 5418, \\\"grader_type\\\": \\\"IN\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"\\\\nIdeas\\\\n3\\\\nContent\\\\n3\\\\nOrganization\\\\n2\\\\nStyle\\\\n2\\\\nVoice\\\\n2\\\"}\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"post_assessment\"}"] # Mock instance state. Should receive a score of 15. MOCK_INSTANCE_STATE = r"""{"ready_to_reset": false, "skip_spelling_checks": true, "current_task_number": 1, "weight": 5.0, "graceperiod": "1 day 12 hours 59 minutes 59 seconds", "graded": "True", "task_states": ["{\"child_created\": false, \"child_attempts\": 4, \"version\": 1, \"child_history\": [{\"answer\": \"After 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"To replicate the experiment, the procedure would require more detail. One piece of information that is omitted is the amount of vinegar used in the experiment. It is also important to know what temperature the experiment was kept at during the 24 hours. Finally, the procedure needs to include details about the experiment, for example if the whole sample must be submerged.\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"e the mass of four different samples.\\r\\nPour vinegar in each of four separate, but identical, containers.\\r\\nPlace a sample of one material into one container and label. Repeat with remaining samples, placing a single sample into a single container.\\r\\nAfter 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"\", \"post_assessment\": \"[3]\", \"score\": 3}], \"max_score\": 3, \"child_state\": \"done\"}", "{\"child_created\": false, \"child_attempts\": 0, \"version\": 1, \"child_history\": [{\"answer\": \"The students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the group\\u2019s procedure, describe what additional information you would need in order to replicate the expe\", \"post_assessment\": \"{\\\"submission_id\\\": 3097, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: More grammar errors than average.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the groups procedure , describe what additional information you would need in order to replicate the expe\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3233, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"After 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the\", \"post_assessment\": \"{\\\"submission_id\\\": 3098, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"after hours , remove the samples from the containers and rinse each sample with distilled water . allow the samples to sit and dry for minutes . determine the mass of each sample . the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3235, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"To replicate the experiment, the procedure would require more detail. One piece of information that is omitted is the amount of vinegar used in the experiment. It is also important to know what temperature the experiment was kept at during the 24 hours. Finally, the procedure needs to include details about the experiment, for example if the whole sample must be submerged.\", \"post_assessment\": \"{\\\"submission_id\\\": 3099, \\\"score\\\": 3, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"to replicate the experiment , the procedure would require more detail . one piece of information that is omitted is the amount of vinegar used in the experiment . it is also important to know what temperature the experiment was kept at during the hours . finally , the procedure needs to include details about the experiment , for example if the whole sample must be submerged .\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3237, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality3\\\"}\", \"score\": 3}, {\"answer\": \"e the mass of four different samples.\\r\\nPour vinegar in each of four separate, but identical, containers.\\r\\nPlace a sample of one material into one container and label. Repeat with remaining samples, placing a single sample into a single container.\\r\\nAfter 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\", \"post_assessment\": \"{\\\"submission_id\\\": 3100, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"e the mass of four different samples . pour vinegar in each of four separate , but identical , containers . place a sample of one material into one container and label . repeat with remaining samples , placing a single sample into a single container . after hours , remove the samples from the containers and rinse each sample with distilled water . allow the samples to sit and dry for minutes . determine the mass of each sample . the students data are recorded in the table below . \\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3239, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"\", \"post_assessment\": \"{\\\"submission_id\\\": 3101, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"invalid essay .\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3241, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}], \"max_score\": 3, \"child_state\": \"done\"}"], "attempts": "10000", "student_attempts": 0, "due": null, "state": "done", "accept_file_upload": false, "display_name": "Science Question -- Machine Assessed"}""" +# Instance state. To test the rubric scores are consistent. Should receive a score of 15. +INSTANCE_INCONSISTENT_STATE = serialize_open_ended_instance_state(""" +{ "accept_file_upload" : false, + "attempts" : "10000", + "current_task_number" : 1, + "display_name" : "Science Question -- Machine Assessed", + "due" : null, + "graceperiod" : "1 day 12 hours 59 minutes 59 seconds", + "graded" : "True", + "ready_to_reset" : false, + "skip_spelling_checks" : true, + "state" : "done", + "student_attempts" : 0, + "task_states" : [ { "child_attempts" : 4, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : [ 3 ], + "score" : 1 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : [ 3 ], + "score" : 1 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : [ 3 ], + "score" : 1 + }, + { "answer" : "", + "post_assessment" : [ 3 ], + "score" : 1 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + }, + { "child_attempts" : 0, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: More grammar errors than average.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3233, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3097, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3235, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3098, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3237, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality3", + "score" : 2, + "submission_id" : 3099, + "success" : true + }, + "score" : 2 + }, + { "answer" : "Student answer 4th attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3239, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3100, + "success" : true + }, + "score" : 0 + }, + { "answer" : "", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "invalid essay .", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3241, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3101, + "success" : true + }, + "score" : 0 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + } + ], + "weight" : 5.0 +} + """) + +# Instance state. Should receive a score of 10 if grader type is PE. +INSTANCE_INCONSISTENT_STATE2 = serialize_open_ended_instance_state(""" +{ "accept_file_upload" : false, + "attempts" : "10000", + "current_task_number" : 1, + "display_name" : "Science Question -- Machine Assessed", + "due" : null, + "graceperiod" : "1 day 12 hours 59 minutes 59 seconds", + "graded" : "True", + "ready_to_reset" : false, + "skip_spelling_checks" : true, + "state" : "done", + "student_attempts" : 0, + "task_states" : [ { "child_attempts" : 4, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "", + "post_assessment" : [3], + "score" : 1 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + }, + { "child_attempts" : 0, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: More grammar errors than average.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3233, + "grader_type" : "PE", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3097, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3235, + "grader_type" : "PE", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3098, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3237, + "grader_type" : "PE", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality5", + "score" : 2, + "submission_id" : 3099, + "success" : true + }, + "score" : 2 + }, + { "answer" : "Student answer 4th attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3239, + "grader_type" : "PE", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3100, + "success" : true + }, + "score" : 0 + }, + { "answer" : "", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "invalid essay .", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3241, + "grader_type" : "PE", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3101, + "success" : true + }, + "score" : 0 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + } + ], + "weight" : 5.0 +} + """) + +# Instance state. To test score if sum of rubric score is different from score value. Should receive score of 25. +INSTANCE_INCONSISTENT_STATE3 = serialize_open_ended_instance_state(""" +{ "accept_file_upload" : false, + "attempts" : "10000", + "current_task_number" : 1, + "display_name" : "Science Question -- Machine Assessed", + "due" : null, + "graceperiod" : "1 day 12 hours 59 minutes 59 seconds", + "graded" : "True", + "ready_to_reset" : false, + "skip_spelling_checks" : true, + "state" : "done", + "student_attempts" : 0, + "task_states" : [ { "child_attempts" : 4, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "", + "post_assessment" : [3], + "score" : 1 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + }, + { "child_attempts" : 0, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: More grammar errors than average.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3233, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality2", + "score" : 0, + "submission_id" : 3097, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3235, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3098, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3237, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality5", + "score" : 2, + "submission_id" : 3099, + "success" : true + }, + "score" : 2 + }, + { "answer" : "Student answer 4th attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3239, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3100, + "success" : true + }, + "score" : 0 + }, + { "answer" : "", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "invalid essay .", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3241, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3101, + "success" : true + }, + "score" : 0 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + } + ], + "weight" : 5.0 +} +""") + +# Instance state. To test score if old task states are available. Should receive a score of 15. +INSTANCE_INCONSISTENT_STATE4 = serialize_open_ended_instance_state(""" +{ "accept_file_upload" : false, + "attempts" : "10000", + "current_task_number" : 0, + "display_name" : "Science Question -- Machine Assessed", + "due" : null, + "graceperiod" : "1 day 12 hours 59 minutes 59 seconds", + "graded" : "True", + "old_task_states" : [ [ { "child_attempts" : 4, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : "[3]", + "score" : 1 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : "[3]", + "score" : 1 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assesssment" : "[3]", + "score" : 1 + }, + { "answer" : "", + "post_assessment" : "[3]", + "score" : 1 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + } ] ], + "ready_to_reset" : false, + "skip_spelling_checks" : true, + "state" : "assessing", + "student_attempts" : 0, + "task_states" : [ { "child_attempts" : 4, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "", + "post_assessment" : [3], + "score" : 1 + } + ], + "child_state" : "done", + "max_score" : 3, + "stored_answer" : null, + "version" : 1 + }, + { "child_attempts" : 0, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: More grammar errors than average.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3233, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3097, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3235, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3098, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3237, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality3", + "score" : 2, + "submission_id" : 3099, + "success" : true + }, + "score" : 2 + }, + { "answer" : "Student answer 4th attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3239, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3100, + "success" : true + }, + "score" : 0 + }, + { "answer" : "", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "invalid essay .", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3241, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3101, + "success" : true + }, + "score" : 0 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + } + ], + "weight" : 5.0 +} +""") + +# Instance state. To test score if rubric scores are available but score is missing. Should receive a score of 15. +INSTANCE_INCONSISTENT_STATE5 = serialize_open_ended_instance_state(""" +{ "accept_file_upload" : false, + "attempts" : "10000", + "current_task_number" : 1, + "display_name" : "Science Question -- Machine Assessed", + "due" : null, + "graceperiod" : "1 day 12 hours 59 minutes 59 seconds", + "graded" : "True", + "ready_to_reset" : false, + "skip_spelling_checks" : true, + "state" : "done", + "student_attempts" : 0, + "task_states" : [ { "child_attempts" : 4, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : [3], + "score" : 1 + }, + { "answer" : "", + "post_assessment" : [3], + "score" : 1 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + }, + { "child_attempts" : 0, + "child_created" : false, + "child_history" : [ { "answer" : "Student answer 1st attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: More grammar errors than average.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3233, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3097, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 2nd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3235, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3098, + "success" : true + }, + "score" : 0 + }, + { "answer" : "Student answer 3rd attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3237, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality3", + "score" : 2, + "submission_id" : 3099, + "success" : true + } + }, + { "answer" : "Student answer 4th attempt.", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "valid essay", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3239, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3100, + "success" : true + }, + "score" : 0 + }, + { "answer" : "", + "post_assessment" : { "feedback" : { "grammar" : "Grammar: Ok.", + "markup-text" : "invalid essay .", + "spelling" : "Spelling: Ok." + }, + "grader_id" : 3241, + "grader_type" : "ML", + "rubric_scores_complete" : true, + "rubric_xml" : "Response Quality0", + "score" : 0, + "submission_id" : 3101, + "success" : true + }, + "score" : 0 + } + ], + "child_state" : "done", + "max_score" : 3, + "version" : 1 + } + ], + "weight" : 5.0 +} +""") + # Task state with self assessment only. TEST_STATE_SA = ["{\"child_created\": false, \"child_attempts\": 1, \"version\": 1, \"child_history\": [{\"answer\": \"Censorship in the Libraries\\r
    'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author\\r

    Write a persuasive essay to a newspaper reflecting your vies on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.\", \"post_assessment\": \"[3, 3, 2, 2, 2]\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"done\"}", "{\"child_created\": false, \"child_attempts\": 0, \"version\": 1, \"child_history\": [{\"answer\": \"Censorship in the Libraries\\r
    'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author\\r

    Write a persuasive essay to a newspaper reflecting your vies on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.\", \"post_assessment\": \"{\\\"submission_id\\\": 1461, \\\"score\\\": 12, \\\"feedback\\\": \\\"{\\\\\\\"feedback\\\\\\\": \\\\\\\"\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 5414, \\\"grader_type\\\": \\\"IN\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"\\\\nIdeas\\\\n3\\\\nContent\\\\n3\\\\nOrganization\\\\n2\\\\nStyle\\\\n2\\\\nVoice\\\\n2\\\"}\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"post_assessment\"}"]