Merge pull request #1028 from edx/aj/fix-empty-file-name-in-response

Fix empty file description in ORA response - EDUCATOR-1054

openassessment/xblock/static/js/spec/lms/oa_response.js

@@ -626,6 +626,10 @@ describe("OpenAssessment.ResponseView", function() {
         function getFileUploadField() {
             return $(view.element).find('.file__upload').first();
         }
+        function expectFileUploadButton(disabled) {
+            view.checkFilesDescriptions();
+            expect(getFileUploadField().is(':disabled')).toEqual(disabled);
+        }
 
         spyOn(view, 'updateFilesDescriptionsFields').and.callThrough();
         var files = [{type: 'image/jpeg', size: 1024, name: 'picture1.jpg', data: ''},
@@ -634,26 +638,36 @@ describe("OpenAssessment.ResponseView", function() {
 
         expect(getFileUploadField().is(':disabled')).toEqual(true);
         expect(view.updateFilesDescriptionsFields).toHaveBeenCalledWith(files, undefined, 'image');
+        var firstDescriptionField1 = $(view.element).find('.file__description__0').first();
+        var firstDescriptionField2 = $(view.element).find('.file__description__1').first();
 
-        // set the first description field (the second is still empty)
+        // Only set the first description field and the second field remain empty.
         // and check that upload button is disabled
-        var firstDescriptionField1 = $(view.element).find('.file__description__0').first();
-        $(firstDescriptionField1).val('test');
-        view.checkFilesDescriptions();
-        expect(getFileUploadField().is(':disabled')).toEqual(true);
+        $(firstDescriptionField1).val('test1');
+        $(firstDescriptionField2).val('');
+        expectFileUploadButton(true);
 
-        // set the second description field (now both descriptions are not empty)
+        // Set the second description field to be only spaces (given first description has value).
+        // and check that upload button is disabled
+        $(firstDescriptionField2).val('  ');
+        expectFileUploadButton(true)
+
+        // Set the both description fields to contain only spaces.
+        // and check that upload button is disabled
+        $(firstDescriptionField1).val(' ');
+        $(firstDescriptionField2).val(' ');
+        expectFileUploadButton(true)
+
+        // set the both description field to contain non empty values.
         // and check that upload button is enabled
-        var firstDescriptionField2 = $(view.element).find('.file__description__1').first();
+        $(firstDescriptionField1).val('test1');
         $(firstDescriptionField2).val('test2');
-        view.checkFilesDescriptions();
-        expect(getFileUploadField().is(':disabled')).toEqual(false);
+        expectFileUploadButton(false)
 
         // remove value in the first upload field
         // and check that upload button is disabled
         $(firstDescriptionField1).val('');
-        view.checkFilesDescriptions();
-        expect(getFileUploadField().is(':disabled')).toEqual(true);
+        expectFileUploadButton(true)
     });
 
     it("removes description fields after files upload", function() {

openassessment/xblock/static/js/src/lms/oa_response.js

@@ -666,7 +666,7 @@ OpenAssessment.ResponseView.prototype = {
         var filesDescriptions = [];
 
         $(this.element).find('.file__description').each(function() {
-            var filesDescriptionVal = $(this).val();
+            var filesDescriptionVal = $.trim($(this).val());
             if (filesDescriptionVal) {
                 filesDescriptions.push(filesDescriptionVal);
             } else {

openassessment/xblock/submission_mixin.py

@@ -400,24 +400,20 @@ class SubmissionMixin(object):
         """
         urls = []
         if 'file_keys' in submission['answer']:
-            keys = submission['answer'].get('file_keys', [])
+            file_keys = submission['answer'].get('file_keys', [])
             descriptions = submission['answer'].get('files_descriptions', [])
-            for idx, key in enumerate(keys):
-                url = self._get_url_by_file_key(key)
-                if url:
-                    description = ''
-                    try:
-                        description = descriptions[idx]
-                    except IndexError:
-                        pass
-                    urls.append((url, description))
+            for idx, key in enumerate(file_keys):
+                file_download_url = self._get_url_by_file_key(key)
+                if file_download_url:
+                    file_description = descriptions[idx].strip() if idx < len(descriptions) else ''
+                    urls.append((file_download_url, file_description))
                 else:
                     break
         elif 'file_key' in submission['answer']:
             key = submission['answer'].get('file_key', '')
-            url = self._get_url_by_file_key(key)
-            if url:
-                urls.append((url, ''))
+            file_download_url = self._get_url_by_file_key(key)
+            if file_download_url:
+                urls.append((file_download_url, ''))
         return urls
 
     @staticmethod

openassessment/xblock/test/test_submission.py

@@ -175,8 +175,6 @@ class SubmissionTest(XBlockHandlerTestCase):
         resp = self.request(xblock, 'download_url', json.dumps(dict()), response_format='json')
 
         self.assertTrue(resp['success'])
-        print download_url
-        print resp['url']
         self.assertEqual(download_url, resp['url'])
 
     @mock_s3
@@ -212,11 +210,10 @@ class SubmissionTest(XBlockHandlerTestCase):
             course_id='test_course',
             anonymous_student_id='test_student',
         )
-
         download_url = api.get_download_url("test_student/test_course/" + xblock.scope_ids.usage_id)
         resp = self.request(xblock, 'download_url', json.dumps(dict()), response_format='json')
         self.assertTrue(resp['success'])
-        self.assertEqual(download_url, resp['url'])
+        self.assertTrue(resp['url'].startswith(download_url))
 
         resp = self.request(xblock, 'remove_all_uploaded_files', json.dumps(dict()), response_format='json')
         self.assertTrue(resp['success'])