diff --git a/lms/djangoapps/ora_staff_grader/ora_staff_grader.postman_collection.json b/lms/djangoapps/ora_staff_grader/ora_staff_grader.postman_collection.json index 22a6573a99..7df454636e 100644 --- a/lms/djangoapps/ora_staff_grader/ora_staff_grader.postman_collection.json +++ b/lms/djangoapps/ora_staff_grader/ora_staff_grader.postman_collection.json @@ -1,7 +1,7 @@ { "info": { - "_postman_id": "da8b1a76-d44c-4f0a-b546-c20a3116f285", - "name": "Enhanced Staff Grader", + "_postman_id": "c92bdcdf-cf1d-4dc8-9e65-740fef3cb41e", + "name": "ORA Staff Grading", "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json" }, "item": [ @@ -770,6 +770,235 @@ } ] }, + { + "name": "Fetch Files", + "event": [ + { + "listen": "prerequest", + "script": { + "exec": [ + "" + ], + "type": "text/javascript" + } + } + ], + "request": { + "method": "GET", + "header": [ + { + "key": "X-CSRFToken", + "value": "{{csrftoken}}", + "type": "text" + } + ], + "url": { + "raw": "{{protocol}}://{{lms_url}}/api/ora_staff_grader{{mock}}/submission/files?oraLocation={{block_id_encoded}}&submissionUUID={{submission_id}}", + "protocol": "{{protocol}}", + "host": [ + "{{lms_url}}" + ], + "path": [ + "api", + "ora_staff_grader{{mock}}", + "submission", + "files" + ], + "query": [ + { + "key": "oraLocation", + "value": "{{block_id_encoded}}", + "description": "ORA location" + }, + { + "key": "submissionUUID", + "value": "{{submission_id}}", + "description": "Individual submission UUID" + }, + { + "key": "oraLocation", + "value": "{{team_block_id_encoded}}", + "description": "Team ORA location", + "disabled": true + }, + { + "key": "submissionUUID", + "value": "{{team_submission_id}}", + "description": "Team submission UUID", + "disabled": true + } + ] + } + }, + "response": [ + { + "name": "Success", + "originalRequest": { + "method": "GET", + "header": [ + { + "key": "X-CSRFToken", + "value": "{{csrftoken}}", + "type": "text" + } + ], + "url": { + "raw": "{{protocol}}://{{lms_url}}/api/ora_staff_grader{{mock}}/submission/files?oraLocation={{block_id_encoded}}&submissionUUID={{submission_id}}", + "protocol": "{{protocol}}", + "host": [ + "{{lms_url}}" + ], + "path": [ + "api", + "ora_staff_grader{{mock}}", + "submission", + "files" + ], + "query": [ + { + "key": "oraLocation", + "value": "{{block_id_encoded}}" + }, + { + "key": "submissionUUID", + "value": "{{submission_id}}" + } + ] + } + }, + "status": "OK", + "code": 200, + "_postman_previewlanguage": "json", + "header": [ + { + "key": "Date", + "value": "Tue, 30 Nov 2021 22:22:28 GMT" + }, + { + "key": "Server", + "value": "WSGIServer/0.2 CPython/3.8.10" + }, + { + "key": "Content-Type", + "value": "application/json" + }, + { + "key": "Vary", + "value": "Accept, Accept-Language, Origin, Cookie" + }, + { + "key": "Allow", + "value": "GET, HEAD, OPTIONS" + }, + { + "key": "Server-Timing", + "value": "TimerPanel_utime;dur=1151.775999999998;desc=\"User CPU time\", TimerPanel_stime;dur=349.742;desc=\"System CPU time\", TimerPanel_total;dur=1501.517999999998;desc=\"Total CPU time\", TimerPanel_total_time;dur=2281.3472747802734;desc=\"Elapsed time\", SQLPanel_sql_time;dur=26.540517807006836;desc=\"SQL 22 queries\"" + }, + { + "key": "X-Frame-Options", + "value": "DENY" + }, + { + "key": "Content-Language", + "value": "en" + }, + { + "key": "Content-Length", + "value": "376" + }, + { + "key": "Set-Cookie", + "value": "openedx-language-preference=en; expires=Tue, 14 Dec 2021 22:22:28 GMT; Max-Age=1209600; Path=/" + } + ], + "cookie": [], + "body": "{\n \"files\": [\n {\n \"downloadUrl\": \"http://localhost:18000/media/submissions_attachments/c84e8e5335234f279676e178044e191c_course-v1%3ADevX%2BORA101%2BT12020_block-v1%3ADevX%2BORA101%2BT12020%2Btype%40openassessment%2Bblock%408c235f76c46948ec80c9d59bf5686d69\",\n \"description\": \"\",\n \"name\": \"my-image.png\",\n \"size\": 3141592\n }\n ]\n}" + }, + { + "name": "Submission not found", + "originalRequest": { + "method": "GET", + "header": [ + { + "key": "X-CSRFToken", + "value": "{{csrftoken}}", + "type": "text" + } + ], + "url": { + "raw": "{{protocol}}://{{lms_url}}/api/ora_staff_grader{{mock}}/submission/files?oraLocation={{block_id_encoded}}&submissionUUID={{submission_id}}", + "protocol": "{{protocol}}", + "host": [ + "{{lms_url}}" + ], + "path": [ + "api", + "ora_staff_grader{{mock}}", + "submission", + "files" + ], + "query": [ + { + "key": "oraLocation", + "value": "{{block_id_encoded}}" + }, + { + "key": "submissionUUID", + "value": "{{submission_id}}" + } + ] + } + }, + "status": "Internal Server Error", + "code": 500, + "_postman_previewlanguage": "json", + "header": [ + { + "key": "Date", + "value": "Wed, 09 Feb 2022 22:31:35 GMT" + }, + { + "key": "Server", + "value": "WSGIServer/0.2 CPython/3.8.10" + }, + { + "key": "Content-Type", + "value": "application/json" + }, + { + "key": "Vary", + "value": "Accept, Accept-Language, Origin, Cookie" + }, + { + "key": "Allow", + "value": "GET, HEAD, OPTIONS" + }, + { + "key": "Server-Timing", + "value": "TimerPanel_utime;dur=874.637;desc=\"User CPU time\", TimerPanel_stime;dur=181.40999999999963;desc=\"System CPU time\", TimerPanel_total;dur=1056.0469999999996;desc=\"Total CPU time\", TimerPanel_total_time;dur=2108.534574508667;desc=\"Elapsed time\", SQLPanel_sql_time;dur=26.390790939331055;desc=\"SQL 19 queries\"" + }, + { + "key": "X-Frame-Options", + "value": "DENY" + }, + { + "key": "Content-Language", + "value": "en" + }, + { + "key": "Content-Length", + "value": "276" + }, + { + "key": "Set-Cookie", + "value": "openedx-language-preference=en; expires=Wed, 23 Feb 2022 22:31:35 GMT; Max-Age=1209600; Path=/; SameSite=Lax" + } + ], + "cookie": [], + "body": "{\n \"error\": \"ERR_INTERNAL\",\n \"handler\": \"get_assessment_info\",\n \"details\": \"No gradeable submission found with uuid=e34ef789-a4b1-48cf-b1bc-b3edacfd4eb2 in course=course-v1:DevX+ORA101+T12020 item=block-v1:DevX+ORA101+T12020+type@openassessment+block@7eebcd59811d4378a000db14f583f070\"\n}" + } + ] + }, { "name": "Fetch Submission Status", "event": [ diff --git a/lms/djangoapps/ora_staff_grader/serializers.py b/lms/djangoapps/ora_staff_grader/serializers.py index 4b0df31021..cf992fc830 100644 --- a/lms/djangoapps/ora_staff_grader/serializers.py +++ b/lms/djangoapps/ora_staff_grader/serializers.py @@ -202,6 +202,12 @@ class ResponseSerializer(serializers.Serializer): text = serializers.ListField(child=serializers.CharField(), allow_empty=True) +class FileListSerializer(serializers.Serializer): + """Serializer for a list of files in a submission""" + + files = serializers.ListField(child=UploadedFileSerializer(), allow_empty=True) + + class AssessmentCriteriaSerializer(serializers.Serializer): """Serializer for information about a criterion, in the context of a completed assessment""" diff --git a/lms/djangoapps/ora_staff_grader/tests/test_serializers.py b/lms/djangoapps/ora_staff_grader/tests/test_serializers.py index b5bc979c1f..5e7362c9c6 100644 --- a/lms/djangoapps/ora_staff_grader/tests/test_serializers.py +++ b/lms/djangoapps/ora_staff_grader/tests/test_serializers.py @@ -11,6 +11,7 @@ from lms.djangoapps.ora_staff_grader.errors import ERR_UNKNOWN, ErrorSerializer from lms.djangoapps.ora_staff_grader.serializers import ( AssessmentCriteriaSerializer, CourseMetadataSerializer, + FileListSerializer, GradeDataSerializer, InitializeSerializer, LockStatusSerializer, @@ -308,7 +309,7 @@ class TestInitializeSerializer(TestCase): """ def set_up_ora(self): - """Create a mock Open Repsponse Assessment for serialization""" + """Create a mock Open Response Assessment for serialization""" ora_data = { "display_name": "Week 1: Time Travel Paradoxes", "prompts": [ @@ -488,6 +489,48 @@ class TestResponseSerializer(TestCase): assert data == expected_value +@ddt.ddt +class TestFileListSerializer(TestCase): + """ + Tests for FileListSerializer - this is basically a stripped down ResponseSerializer + """ + + def test_file_list_serializer__empty(self): + """Empty fields should be allowed""" + input_data = {"files": [], "text": []} + expected_output = {"files": []} + assert FileListSerializer(input_data).data == expected_output + + def test_file_list_serializer(self): + """Base serialization behavior""" + input_data = { + "files": [{ + "name": Mock(), + "description": Mock(), + "download_url": Mock(), + "size": 12345, + }, { + "name": Mock(), + "description": Mock(), + "download_url": Mock(), + "size": 54321, + }], + "text": "", + } + + output_data = FileListSerializer(input_data).data + assert output_data.keys() == set(["files"]) + + for i, input_file in enumerate(input_data["files"]): + output_file = output_data["files"][i] + assert output_file.keys() == set(["name", "description", "downloadUrl", "size"]) + + assert output_file["name"] == str(input_file["name"]) + assert output_file["description"] == str(input_file["description"]) + assert output_file["downloadUrl"] == str(input_file["download_url"]) + assert output_file["size"] == input_file["size"] + + class TestAssessmentCriteriaSerializer(TestCase): """Tests for AssessmentCriteriaSerializer""" @@ -507,7 +550,7 @@ class TestAssessmentCriteriaSerializer(TestCase): def test_assessment_criteria_serializer__feedback_only(self): """Test for serialization behavior of a feedback-only criterion""" input_data = { - "name": "SomeCriterioOn", + "name": "SomeCriterion", "feedback": "Pathetic Effort", "points": None, "option": None, diff --git a/lms/djangoapps/ora_staff_grader/tests/test_views.py b/lms/djangoapps/ora_staff_grader/tests/test_views.py index d9ad6c4a4f..aacceda19a 100644 --- a/lms/djangoapps/ora_staff_grader/tests/test_views.py +++ b/lms/djangoapps/ora_staff_grader/tests/test_views.py @@ -219,13 +219,13 @@ class TestFetchSubmissionView(BaseViewTest): @patch("lms.djangoapps.ora_staff_grader.views.get_submission_info") @patch("lms.djangoapps.ora_staff_grader.views.get_assessment_info") @patch("lms.djangoapps.ora_staff_grader.views.check_submission_lock") - def test_fetch_submission_generic_exception( + def test_fetch_submission_xblock_exception( self, mock_check_submission_lock, mock_get_assessment_info, mock_get_submission_info, ): - """Other generic exceptions should return the "unknown" error response""" + """An exception in any XBlock handler returns an error response""" mock_get_submission_info.return_value = test_data.example_submission # Mock an error in getting the assessment info mock_get_assessment_info.side_effect = XBlockInternalError( @@ -248,13 +248,13 @@ class TestFetchSubmissionView(BaseViewTest): @patch("lms.djangoapps.ora_staff_grader.views.get_submission_info") @patch("lms.djangoapps.ora_staff_grader.views.get_assessment_info") @patch("lms.djangoapps.ora_staff_grader.views.check_submission_lock") - def test_fetch_submission_xblock_exception( + def test_fetch_submission_generic_exception( self, mock_check_submission_lock, mock_get_assessment_info, mock_get_submission_info, ): - """An exception in any XBlock handler returns an error response""" + """Other generic exceptions should return the "unknown" error response""" mock_get_submission_info.return_value = test_data.example_submission mock_get_assessment_info.return_value = test_data.example_assessment # Mock a bad data shape to break serialization @@ -270,6 +270,75 @@ class TestFetchSubmissionView(BaseViewTest): assert json.loads(response.content) == {"error": ERR_UNKNOWN} +@ddt.ddt +class TestFilesFetchView(BaseViewTest): + """ + Tests for the SubmissionFilesFetchView + """ + + view_name = "ora-staff-grader:fetch-files" + + def setUp(self): + super().setUp() + self.log_in() + + @ddt.data({}, {PARAM_ORA_LOCATION: "", PARAM_SUBMISSION_ID: ""}) + def test_missing_params(self, query_params): + """Missing or blank params should return 400 and error message""" + response = self.client.get(self.api_url, query_params) + + assert response.status_code == 400 + assert json.loads(response.content) == {"error": ERR_MISSING_PARAM} + + @patch("lms.djangoapps.ora_staff_grader.views.get_submission_info") + def test_fetch_files(self, mock_get_submission_info): + """Successfull file fetch returns the list of files for a submission""" + mock_get_submission_info.return_value = test_data.example_submission + + ora_location, submission_uuid = Mock(), Mock() + response = self.client.get( + self.api_url, + {PARAM_ORA_LOCATION: ora_location, PARAM_SUBMISSION_ID: submission_uuid}, + ) + + assert response.status_code == 200 + assert response.data.keys() == set(["files"]) + assert len(test_data.example_submission["files"]) == len(response.data['files']) + + @patch("lms.djangoapps.ora_staff_grader.views.get_submission_info") + def test_fetch_files_generic_exception(self, mock_get_submission_info): + """Other generic exceptions should return the "unknown" error response""" + mock_get_submission_info.side_effect = Exception() + + ora_location, submission_uuid = Mock(), Mock() + response = self.client.get( + self.api_url, + {PARAM_ORA_LOCATION: ora_location, PARAM_SUBMISSION_ID: submission_uuid}, + ) + + assert response.status_code == 500 + assert json.loads(response.content) == {"error": ERR_UNKNOWN} + + @patch("lms.djangoapps.ora_staff_grader.views.get_submission_info") + def test_fetch_files_xblock_exception(self, mock_get_submission_info): + """An exception in any XBlock handler returns an error response""" + mock_get_submission_info.side_effect = XBlockInternalError( + context={"handler": "get_submission_info"} + ) + + ora_location, submission_uuid = Mock(), Mock() + response = self.client.get( + self.api_url, + {PARAM_ORA_LOCATION: ora_location, PARAM_SUBMISSION_ID: submission_uuid}, + ) + + assert response.status_code == 500 + assert json.loads(response.content) == { + "error": ERR_INTERNAL, + "handler": "get_submission_info", + } + + @ddt.ddt class TestFetchSubmissionStatusView(BaseViewTest): """ @@ -343,7 +412,6 @@ class TestFetchSubmissionStatusView(BaseViewTest): self, mock_check_submission_lock, mock_get_assessment_info ): """Exceptions within an XBlock return an internal error response""" - # Mock a bad data shape to throw a serializer exception mock_get_assessment_info.return_value = {} mock_check_submission_lock.side_effect = XBlockInternalError( context={"handler": "claim_submission_lock"} @@ -482,7 +550,7 @@ class TestSubmissionLockView(BaseViewTest): mock_claim_lock, ): """In the even more unlikely event of an unhandled error, shrug exuberantly""" - # Mock a bad data shape to break serialiation and raise a generic exception + # Mock a bad data shape to break serialization and raise a generic exception mock_claim_lock.return_value = {"android": "Rachel"} response = self.claim_lock(self.test_lock_params) @@ -539,7 +607,7 @@ class TestSubmissionLockView(BaseViewTest): @patch("lms.djangoapps.ora_staff_grader.views.delete_submission_lock") def test_delete_lock_generic_exception(self, mock_delete_lock): """In the even more unlikely event of an unhandled error, shrug exuberantly""" - # Mock a bad data shape to break serialiation and raise a generic exception + # Mock a bad data shape to break serialization and raise a generic exception mock_delete_lock.return_value = {"android": "Roy Batty"} response = self.delete_lock(self.test_lock_params) @@ -591,7 +659,7 @@ class TestBatchSubmissionLockView(BaseViewTest): mock_batch_delete.assert_not_called() @patch("lms.djangoapps.ora_staff_grader.views.batch_delete_submission_locks") - def test_batch_unlock_missing_submisison_list(self, mock_batch_delete): + def test_batch_unlock_missing_submission_list(self, mock_batch_delete): """An invalid ORA returns a 400""" response = self.batch_unlock(self.test_request_params, {}) diff --git a/lms/djangoapps/ora_staff_grader/urls.py b/lms/djangoapps/ora_staff_grader/urls.py index c5e3a2a837..f4b0a39b19 100644 --- a/lms/djangoapps/ora_staff_grader/urls.py +++ b/lms/djangoapps/ora_staff_grader/urls.py @@ -9,6 +9,7 @@ from lms.djangoapps.ora_staff_grader.views import ( InitializeView, SubmissionBatchUnlockView, SubmissionFetchView, + SubmissionFilesFetchView, SubmissionLockView, SubmissionStatusFetchView, UpdateGradeView, @@ -22,6 +23,7 @@ urlpatterns += [ path("mock/", include("lms.djangoapps.ora_staff_grader.mock.urls")), path("initialize", InitializeView.as_view(), name="initialize"), path("submission/batch/unlock", SubmissionBatchUnlockView.as_view(), name="batch-unlock"), + path("submission/files", SubmissionFilesFetchView.as_view(), name="fetch-files"), path( "submission/status", SubmissionStatusFetchView.as_view(), diff --git a/lms/djangoapps/ora_staff_grader/views.py b/lms/djangoapps/ora_staff_grader/views.py index c14a208c31..cf4d08fbc7 100644 --- a/lms/djangoapps/ora_staff_grader/views.py +++ b/lms/djangoapps/ora_staff_grader/views.py @@ -46,6 +46,7 @@ from lms.djangoapps.ora_staff_grader.ora_api import ( submit_grade, ) from lms.djangoapps.ora_staff_grader.serializers import ( + FileListSerializer, InitializeSerializer, LockStatusSerializer, StaffAssessSerializer, @@ -268,6 +269,51 @@ class SubmissionStatusFetchView(StaffGraderBaseView): return UnknownErrorResponse() +class SubmissionFilesFetchView(StaffGraderBaseView): + """ + GET file metadata for a submission. + + Used to get updated file download links to avoid signed download link expiration + issues. + + Response: { + files: [ + downloadUrl (url), + description (string), + name (string), + size (bytes), + ] + } + + Errors: + - MissingParamResponse (HTTP 400) for missing params + - XBlockInternalError (HTTP 500) for an issue with ORA + - UnknownError (HTTP 500) for other errors + """ + + @require_params([PARAM_ORA_LOCATION, PARAM_SUBMISSION_ID]) + def get(self, request, ora_location, submission_uuid, *args, **kwargs): + try: + submission_info = get_submission_info( + request, ora_location, submission_uuid + ) + + response_data = FileListSerializer(submission_info).data + + log.info(response_data) + return Response(response_data) + + # Issues with the XBlock handlers + except XBlockInternalError as ex: + log.error(ex) + return InternalErrorResponse(context=ex.context) + + # Blanket exception handling in case something blows up + except Exception as ex: + log.exception(ex) + return UnknownErrorResponse() + + class UpdateGradeView(StaffGraderBaseView): """ POST submit a grade for a submission @@ -327,7 +373,7 @@ class UpdateGradeView(StaffGraderBaseView): log.error(f"Grade contested for submission: {submission_uuid}") return GradeContestedResponse(context=submission_status) - # Transform grade data and submit assessment, rasies on failure + # Transform grade data and submit assessment, raises on failure context = {"submission_uuid": submission_uuid} grade_data = StaffAssessSerializer(request.data, context=context).data submit_grade(request, ora_location, grade_data)