From 25d3890595a38cdea65550e54a8eb57aada103cb Mon Sep 17 00:00:00 2001 From: kimth Date: Sat, 18 Aug 2012 09:51:27 -0400 Subject: [PATCH 01/13] Remove outdated hardcoded url from circuit simulator --- common/lib/xmodule/xmodule/js/src/capa/schematic.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/lib/xmodule/xmodule/js/src/capa/schematic.js b/common/lib/xmodule/xmodule/js/src/capa/schematic.js index 56c4bc8195..259a089aca 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/schematic.js +++ b/common/lib/xmodule/xmodule/js/src/capa/schematic.js @@ -172,11 +172,13 @@ schematic = (function() { this.tools = new Array(); this.toolbar = []; + /* DISABLE HELP BUTTON -- SJSU if (!this.diagram_only) { this.tools['help'] = this.add_tool(help_icon,'Help: display help page',this.help); this.enable_tool('help',true); this.toolbar.push(null); // spacer } + END DISABLE HELP BUTTON -- SJSU */ if (this.edits_allowed) { this.tools['grid'] = this.add_tool(grid_icon,'Grid: toggle grid display',this.toggle_grid); From 78b3ca050ecf653a1bf994f50f361b4b4869e9fa Mon Sep 17 00:00:00 2001 From: kimth Date: Sat, 18 Aug 2012 10:08:49 -0400 Subject: [PATCH 02/13] More info in comment --- common/lib/xmodule/xmodule/js/src/capa/schematic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/schematic.js b/common/lib/xmodule/xmodule/js/src/capa/schematic.js index 259a089aca..92bc32441b 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/schematic.js +++ b/common/lib/xmodule/xmodule/js/src/capa/schematic.js @@ -172,7 +172,7 @@ schematic = (function() { this.tools = new Array(); this.toolbar = []; - /* DISABLE HELP BUTTON -- SJSU + /* DISABLE HELP BUTTON (target URL not consistent with multicourse hierarchy) -- SJSU if (!this.diagram_only) { this.tools['help'] = this.add_tool(help_icon,'Help: display help page',this.help); this.enable_tool('help',true); From 588444c46bc29134ec5c02d94255ebd5246f42fd Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 07:54:10 -0400 Subject: [PATCH 03/13] Filesubmission template allows multiple file selection --- common/lib/capa/capa/templates/filesubmission.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/templates/filesubmission.html b/common/lib/capa/capa/templates/filesubmission.html index d3d57ee318..fccf469015 100644 --- a/common/lib/capa/capa/templates/filesubmission.html +++ b/common/lib/capa/capa/templates/filesubmission.html @@ -1,5 +1,5 @@
-
+
% if state == 'unsubmitted': % elif state == 'correct': From d920e718d0c3714a09ff28706745b9f91f1343c7 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 08:26:12 -0400 Subject: [PATCH 04/13] AJAX permits multiple file upload --- .../xmodule/xmodule/js/src/capa/display.coffee | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index c00b680eba..0db000188e 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -157,17 +157,19 @@ class @Problem fd = new FormData() # Sanity check of file size - file_too_large = false + abort_submission = false max_filesize = 4*1000*1000 # 4 MB @inputs.each (index, element) -> if element.type is 'file' - if element.files[0] instanceof File - if element.files[0].size > max_filesize - file_too_large = true - alert 'Submission aborted! Your file "' + element.files[0].name + '" is too large (max size: ' + max_filesize/(1000*1000) + ' MB)' - fd.append(element.id, element.files[0]) - else + for file in element.files + if file.size > max_filesize + abort_submission = true + alert 'Submission aborted! Your file "' + file.name '" is too large (max size: ' + max_filesize/(1000*1000) + ' MB)' + fd.append(element.id, file) + if element.files.length == 0 + abort_submission = true + alert 'Submission aborted! You did not select any files to submit' fd.append(element.id, '') else fd.append(element.id, element.value) @@ -185,7 +187,7 @@ class @Problem else alert(response.success) - if not file_too_large + if not abort_submission $.ajaxWithPrefix("#{@url}/problem_check", settings) check: => From d2cb6458348f581eba9dccfcf7ea573c8a139fee Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 09:26:03 -0400 Subject: [PATCH 05/13] Multiple file submissions --- common/lib/capa/capa/responsetypes.py | 16 +++++++------- common/lib/capa/capa/util.py | 18 ++++++++++++++-- common/lib/capa/capa/xqueue_interface.py | 15 ++++++------- .../xmodule/js/src/capa/display.coffee | 21 ++++++++++++------- lms/djangoapps/courseware/module_render.py | 17 ++++++++------- 5 files changed, 54 insertions(+), 33 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 3a2e40f896..cc67389da9 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1119,11 +1119,6 @@ class CodeResponse(LoncapaResponse): (err, self.answer_id, convert_files_to_filenames(student_answers))) raise Exception(err) - if is_file(submission): - self.context.update({'submission': submission.name}) - else: - self.context.update({'submission': submission}) - # Prepare xqueue request #------------------------------------------------------------ qinterface = self.system.xqueue['interface'] @@ -1135,14 +1130,19 @@ class CodeResponse(LoncapaResponse): queue_name=self.queue_name) # Generate body + if is_list_of_files(submission): + self.context.update({'submission': queuekey}) # For tracking. TODO: May want to record something else here + else: + self.context.update({'submission': submission}) + contents = self.payload.copy() # Submit request. When successful, 'msg' is the prior length of the queue - if is_file(submission): - contents.update({'student_response': submission.name}) + if is_list_of_files(submission): + contents.update({'student_response': ''}) # TODO: Is there any information we want to send here? (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents), - file_to_upload=submission) + files_to_upload=submission) else: contents.update({'student_response': submission}) (error, msg) = qinterface.send_to_queue(header=xheader, diff --git a/common/lib/capa/capa/util.py b/common/lib/capa/capa/util.py index 005494e8c0..01f5fe4c82 100644 --- a/common/lib/capa/capa/util.py +++ b/common/lib/capa/capa/util.py @@ -39,12 +39,26 @@ def convert_files_to_filenames(answers): ''' new_answers = dict() for answer_id in answers.keys(): - if is_file(answers[answer_id]): - new_answers[answer_id] = answers[answer_id].name + answer = answers[answer_id] + if is_list_of_files(answer): # Files are stored as a list, even if one file + list_of_filenames = [] + for inputfile in answer: + list_of_filenames.append(inputfile.name) + new_answers[answer_id] = list_of_filenames else: new_answers[answer_id] = answers[answer_id] return new_answers +def is_list_of_files(list_of_files_to_test): + if not isinstance(list_of_files_to_test, list): + return False + + for li in list_of_files_to_test: + if not is_file(li): + return False + + return True + def is_file(file_to_test): ''' Duck typing to check if 'file_to_test' is a File object diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 2847968a89..81dac22936 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -65,7 +65,7 @@ class XQueueInterface(object): self.auth = django_auth self.session = requests.session(auth=requests_auth) - def send_to_queue(self, header, body, file_to_upload=None): + def send_to_queue(self, header, body, files_to_upload=[]): ''' Submit a request to xqueue. @@ -74,16 +74,16 @@ class XQueueInterface(object): body: Serialized data for the receipient behind the queueing service. The operation of xqueue is agnostic to the contents of 'body' - file_to_upload: File object to be uploaded to xqueue along with queue request + files_to_upload: List of file objects to be uploaded to xqueue along with queue request Returns (error_code, msg) where error_code != 0 indicates an error ''' # Attempt to send to queue - (error, msg) = self._send_to_queue(header, body, file_to_upload) + (error, msg) = self._send_to_queue(header, body, files_to_upload) if error and (msg == 'login_required'): # Log in, then try again self._login() - (error, msg) = self._send_to_queue(header, body, file_to_upload) + (error, msg) = self._send_to_queue(header, body, files_to_upload) return (error, msg) @@ -94,12 +94,13 @@ class XQueueInterface(object): return self._http_post(self.url+'/xqueue/login/', payload) - def _send_to_queue(self, header, body, file_to_upload=None): + def _send_to_queue(self, header, body, files_to_upload): payload = {'xqueue_header': header, 'xqueue_body' : body} files = None - if file_to_upload is not None: - files = { file_to_upload.name: file_to_upload } + for f in files_to_upload: + files = { f.name: f } + return self._http_post(self.url+'/xqueue/submit/', payload, files) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index 0db000188e..a242757357 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -151,28 +151,33 @@ class @Problem return if not window.FormData - alert "Sorry, your browser does not support file uploads. Your submit request could not be fulfilled. If you can, please use Chrome or Safari which have been verified to support file uploads." + alert "Submission aborted! Sorry, your browser does not support file uploads. If you can, please use Chrome or Safari which have been verified to support file uploads." return fd = new FormData() - # Sanity check of file size - abort_submission = false + # Sanity checks on submission max_filesize = 4*1000*1000 # 4 MB + file_too_large = false + file_not_selected = false @inputs.each (index, element) -> if element.type is 'file' for file in element.files if file.size > max_filesize - abort_submission = true + file_too_large = true alert 'Submission aborted! Your file "' + file.name '" is too large (max size: ' + max_filesize/(1000*1000) + ' MB)' fd.append(element.id, file) if element.files.length == 0 - abort_submission = true - alert 'Submission aborted! You did not select any files to submit' - fd.append(element.id, '') + file_not_selected = true + fd.append(element.id, '') # In case we want to allow submissions with no file else fd.append(element.id, element.value) + + if file_not_selected + alert 'Submission aborted! You did not select any files to submit' + + abort_submission = file_too_large or file_not_selected settings = type: "POST" @@ -186,7 +191,7 @@ class @Problem @updateProgress response else alert(response.success) - + if not abort_submission $.ajaxWithPrefix("#{@url}/problem_check", settings) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 558f6deeb2..53c7e453dd 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -375,15 +375,16 @@ def modx_dispatch(request, dispatch=None, id=None, course_id=None): # ''' (fix emacs broken parsing) # Check for submitted files and basic file size checks - p = request.POST.copy() + p = request.POST.dict() if request.FILES: - for inputfile_id in request.FILES.keys(): - inputfile = request.FILES[inputfile_id] - if inputfile.size > settings.STUDENT_FILEUPLOAD_MAX_SIZE: # Bytes - file_too_big_msg = 'Submission aborted! Your file "%s" is too large (max size: %d MB)' %\ - (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE/(1000**2)) - return HttpResponse(json.dumps({'success': file_too_big_msg})) - p[inputfile_id] = inputfile + for fileinput_id in request.FILES.keys(): + inputfiles = request.FILES.getlist(fileinput_id) + for inputfile in inputfiles: + if inputfile.size > settings.STUDENT_FILEUPLOAD_MAX_SIZE: # Bytes + file_too_big_msg = 'Submission aborted! Your file "%s" is too large (max size: %d MB)' %\ + (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE/(1000**2)) + return HttpResponse(json.dumps({'success': file_too_big_msg})) + p[fileinput_id] = inputfiles student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, modulestore().get_item(id)) instance = get_module(request.user, request, id, student_module_cache, course_id=course_id) From b2599f075f1a1a8c275aa7bcbb52c2c3d307576f Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 10:57:38 -0400 Subject: [PATCH 06/13] Send multiple files to xqueue --- common/lib/capa/capa/xqueue_interface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 81dac22936..be0c3aa7ca 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -99,7 +99,7 @@ class XQueueInterface(object): 'xqueue_body' : body} files = None for f in files_to_upload: - files = { f.name: f } + files.update({ f.name: f }) return self._http_post(self.url+'/xqueue/submit/', payload, files) From f5e60412ee3fb3c9e73d48126534b2903ccd6c77 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 11:02:03 -0400 Subject: [PATCH 07/13] Send multiple files to xqueue --- common/lib/capa/capa/xqueue_interface.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index be0c3aa7ca..40c88b72f8 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -97,11 +97,11 @@ class XQueueInterface(object): def _send_to_queue(self, header, body, files_to_upload): payload = {'xqueue_header': header, 'xqueue_body' : body} - files = None + files = {} for f in files_to_upload: files.update({ f.name: f }) - return self._http_post(self.url+'/xqueue/submit/', payload, files) + return self._http_post(self.url+'/xqueue/submit/', payload, files=files) def _http_post(self, url, data, files=None): From 52d243208eb5383ca8bee78c4f15ed93ac5382d1 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 11:14:32 -0400 Subject: [PATCH 08/13] Adjust tests to reflect list of file objects, not just file --- common/lib/xmodule/xmodule/tests/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index f08b0f8d6e..a3eac0258e 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -341,11 +341,11 @@ class CodeResponseTest(unittest.TestCase): fp = open(problem_file) answers_with_file = {'1_2_1': 'String-based answer', '1_3_1': ['answer1', 'answer2', 'answer3'], - '1_4_1': fp} + '1_4_1': [fp, fp]} answers_converted = convert_files_to_filenames(answers_with_file) self.assertEquals(answers_converted['1_2_1'], 'String-based answer') self.assertEquals(answers_converted['1_3_1'], ['answer1', 'answer2', 'answer3']) - self.assertEquals(answers_converted['1_4_1'], fp.name) + self.assertEquals(answers_converted['1_4_1'], [fp.name, fp.name]) class ChoiceResponseTest(unittest.TestCase): From e2bca44ca3cba1106b1f3e355b74e23a2877451a Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 12:09:38 -0400 Subject: [PATCH 09/13] Simplify is_list_of_files --- common/lib/capa/capa/util.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/common/lib/capa/capa/util.py b/common/lib/capa/capa/util.py index 01f5fe4c82..a88c45a335 100644 --- a/common/lib/capa/capa/util.py +++ b/common/lib/capa/capa/util.py @@ -49,15 +49,8 @@ def convert_files_to_filenames(answers): new_answers[answer_id] = answers[answer_id] return new_answers -def is_list_of_files(list_of_files_to_test): - if not isinstance(list_of_files_to_test, list): - return False - - for li in list_of_files_to_test: - if not is_file(li): - return False - - return True +def is_list_of_files(files): + return isinstance(files, list) and all(is_file(f) for f in files) def is_file(file_to_test): ''' From 9b94bb17e4fd78c1b2da3eb9b18753160a3cea35 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 12:19:10 -0400 Subject: [PATCH 10/13] Default param is None rather than [] --- common/lib/capa/capa/xqueue_interface.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 40c88b72f8..10f36636b8 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -65,7 +65,7 @@ class XQueueInterface(object): self.auth = django_auth self.session = requests.session(auth=requests_auth) - def send_to_queue(self, header, body, files_to_upload=[]): + def send_to_queue(self, header, body, files_to_upload=None): ''' Submit a request to xqueue. @@ -98,8 +98,9 @@ class XQueueInterface(object): payload = {'xqueue_header': header, 'xqueue_body' : body} files = {} - for f in files_to_upload: - files.update({ f.name: f }) + if files_to_upload is not None: + for f in files_to_upload: + files.update({ f.name: f }) return self._http_post(self.url+'/xqueue/submit/', payload, files=files) From 20adaa81bae1f9b6eb93e7df9ed0f42c543b5dc5 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 12:22:14 -0400 Subject: [PATCH 11/13] Use list comprehension --- common/lib/capa/capa/util.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/common/lib/capa/capa/util.py b/common/lib/capa/capa/util.py index a88c45a335..d12499ee40 100644 --- a/common/lib/capa/capa/util.py +++ b/common/lib/capa/capa/util.py @@ -41,10 +41,7 @@ def convert_files_to_filenames(answers): for answer_id in answers.keys(): answer = answers[answer_id] if is_list_of_files(answer): # Files are stored as a list, even if one file - list_of_filenames = [] - for inputfile in answer: - list_of_filenames.append(inputfile.name) - new_answers[answer_id] = list_of_filenames + new_answers[answer_id] = [f.name for f in answer] else: new_answers[answer_id] = answers[answer_id] return new_answers From 2f63a9b4032521c6e1180398be1af5904911109c Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 20:12:01 -0400 Subject: [PATCH 12/13] Limit number of files that can be uploaded at once --- lms/djangoapps/courseware/module_render.py | 6 ++++++ lms/envs/common.py | 1 + 2 files changed, 7 insertions(+) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 53c7e453dd..7a23927504 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -379,6 +379,12 @@ def modx_dispatch(request, dispatch=None, id=None, course_id=None): if request.FILES: for fileinput_id in request.FILES.keys(): inputfiles = request.FILES.getlist(fileinput_id) + + if len(inputfiles) > settings.MAX_FILEUPLOADS_PER_INPUT: + too_many_files_msg = 'Submission aborted! Maximum %d files may be submitted at once' %\ + settings.MAX_FILEUPLOADS_PER_INPUT + return HttpResponse(json.dumps({'success': too_many_files_msg})) + for inputfile in inputfiles: if inputfile.size > settings.STUDENT_FILEUPLOAD_MAX_SIZE: # Bytes file_too_big_msg = 'Submission aborted! Your file "%s" is too large (max size: %d MB)' %\ diff --git a/lms/envs/common.py b/lms/envs/common.py index c412a3c8cd..14a9627b40 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -141,6 +141,7 @@ TEMPLATE_CONTEXT_PROCESSORS = ( ) STUDENT_FILEUPLOAD_MAX_SIZE = 4*1000*1000 # 4 MB +MAX_FILEUPLOADS_PER_INPUT = 10 # FIXME: # We should have separate S3 staged URLs in case we need to make changes to From 891797a3bb7e58df27c27dda1ceb82420711a57a Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 19 Aug 2012 20:38:10 -0400 Subject: [PATCH 13/13] Submission of files through requests.POST depletes the file pointer. Need to rewind for replay --- common/lib/capa/capa/xqueue_interface.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 10f36636b8..2930eb682d 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -83,6 +83,9 @@ class XQueueInterface(object): if error and (msg == 'login_required'): # Log in, then try again self._login() + if files_to_upload is not None: + for f in files_to_upload: # Need to rewind file pointers + f.seek(0) (error, msg) = self._send_to_queue(header, body, files_to_upload) return (error, msg)