Merge pull request #103 from amir-qayyum-khan/500response_handling

Handle file not found error on SGA

Author: pdpinch

edx_sga/sga.py

@@ -491,7 +491,12 @@ def staff_download(self, request, suffix=''):
         submission = self.get_submission(request.params['student_id'])
         answer = submission['answer']
         path = self._file_storage_path(answer['sha1'], answer['filename'])
-        return self.download(path, answer['mimetype'], answer['filename'])
+        return self.download(
+            path,
+            answer['mimetype'],
+            answer['filename'],
+            require_staff=True
+        )
 
     @XBlock.handler
     def staff_download_annotated(self, request, suffix=''):
@@ -509,20 +514,36 @@ def staff_download_annotated(self, request, suffix=''):
         return self.download(
             path,
             state['annotated_mimetype'],
-            state['annotated_filename']
+            state['annotated_filename'],
+            require_staff=True
         )
 
-    def download(self, path, mime_type, filename):
+    def download(self, path, mime_type, filename, require_staff=False):
         """
         Return a file from storage and return in a Response.
         """
-        file_descriptor = default_storage.open(path)
-        app_iter = iter(partial(file_descriptor.read, BLOCK_SIZE), '')
-        return Response(
-            app_iter=app_iter,
-            content_type=mime_type,
-            content_disposition=("attachment; filename=" +
-                                 filename.encode('utf-8')))
+        try:
+            file_descriptor = default_storage.open(path)
+            app_iter = iter(partial(file_descriptor.read, BLOCK_SIZE), '')
+            return Response(
+                app_iter=app_iter,
+                content_type=mime_type,
+                content_disposition="attachment; filename=" + filename.encode('utf-8'))
+        except IOError:
+            if require_staff:
+                return Response(
+                    "Sorry, assignment {} cannot be found at"
+                    " {}. Please contact {}".format(
+                        filename.encode('utf-8'), path, settings.TECH_SUPPORT_EMAIL
+                    ),
+                    status_code=404
+                )
+            return Response(
+                "Sorry, the file you uploaded, {}, cannot be"
+                " found. Please try uploading it again or contact"
+                " course staff".format(filename.encode('utf-8')),
+                status_code=404
+            )
 
     @XBlock.handler
     def get_staff_grading_data(self, request, suffix=''):

edx_sga/tests.py

@@ -10,6 +10,7 @@
 import pytz
 import tempfile
 import unittest
+from mock import patch
 
 from courseware.models import StudentModule
 from django.contrib.auth.models import User
@@ -396,6 +397,13 @@ def test_upload_download_assignment(self):
         response = block.download_assignment(None)
         self.assertEqual(response.body, expected)
 
+        with patch(
+            "edx_sga.sga.StaffGradedAssignmentXBlock._file_storage_path",
+            return_value=block._file_storage_path("", "test_notfound.txt")
+        ):
+            response = block.download_assignment(None)
+            self.assertEqual(response.status_code, 404)
+
     def test_staff_upload_download_annotated(self):
         # pylint: disable=no-member
         """
@@ -413,6 +421,14 @@ def test_staff_upload_download_annotated(self):
             'module_id': fred.id}))
         self.assertEqual(response.body, expected)
 
+        with patch(
+            "edx_sga.sga.StaffGradedAssignmentXBlock._file_storage_path",
+            return_value=block._file_storage_path("", "test_notfound.txt")
+        ):
+            response = block.staff_download_annotated(mock.Mock(params={
+            'module_id': fred.id}))
+            self.assertEqual(response.status_code, 404)
+
     def test_download_annotated(self):
         # pylint: disable=no-member
         """
@@ -430,6 +446,13 @@ def test_download_annotated(self):
         response = block.download_annotated(None)
         self.assertEqual(response.body, expected)
 
+        with patch(
+            "edx_sga.sga.StaffGradedAssignmentXBlock._file_storage_path",
+            return_value=block._file_storage_path("", "test_notfound.txt")
+        ):
+            response = block.download_annotated(None)
+            self.assertEqual(response.status_code, 404)
+
     def test_staff_download(self):
         """
         Test download for staff.
@@ -445,6 +468,14 @@ def test_staff_download(self):
             'student_id': student['item'].student_id}))
         self.assertEqual(response.body, expected)
 
+        with patch(
+            "edx_sga.sga.StaffGradedAssignmentXBlock._file_storage_path",
+            return_value=block._file_storage_path("", "test_notfound.txt")
+        ):
+            response = block.staff_download(mock.Mock(params={
+            'student_id': student['item'].student_id}))
+            self.assertEqual(response.status_code, 404)
+
     def test_download_annotated_unicode_filename(self):
         """
         Tests download annotated assignment
@@ -462,6 +493,13 @@ def test_download_annotated_unicode_filename(self):
         response = block.download_annotated(None)
         self.assertEqual(response.body, expected)
 
+        with patch(
+            "edx_sga.sga.StaffGradedAssignmentXBlock._file_storage_path",
+            return_value=block._file_storage_path("", "test_notfound.txt")
+        ):
+            response = block.download_annotated(None)
+            self.assertEqual(response.status_code, 404)
+
     def test_staff_download_unicode_filename(self):
         """
         Tests download assignment with filename in unicode for staff.
@@ -476,6 +514,13 @@ def test_staff_download_unicode_filename(self):
         response = block.staff_download(mock.Mock(params={
             'student_id': student['item'].student_id}))
         self.assertEqual(response.body, expected)
+        with patch(
+            "edx_sga.sga.StaffGradedAssignmentXBlock._file_storage_path",
+            return_value=block._file_storage_path("", "test_notfound.txt")
+        ):
+            response = block.staff_download(mock.Mock(params={
+            'student_id': student['item'].student_id}))
+            self.assertEqual(response.status_code, 404)
 
     def test_get_staff_grading_data_not_staff(self):
         """