Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update initialize endpoint and create assessments/feedback endpoint in ORA Staff Grader #33632

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions lms/djangoapps/ora_staff_grader/ora_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
These are checked (usually by checking for a {"success":false} response) and raise errors, possibly with extra context.
"""
import json
from http import HTTPStatus

from rest_framework.request import Request

from lms.djangoapps.ora_staff_grader.errors import (
LockContestedError,
XBlockInternalError,
Expand All @@ -33,6 +37,42 @@ def get_submissions(request, usage_id):
return json.loads(response.content)


def get_assessments(request: Request, usage_id: str, handler_name: str, submission_uuid: str):
"""
Get a list of assessments according to the handler name from ORA XBlock.json_handler

* `list_assessments_to` handler
Lists all assessments given by a user (according to their submissionUUID)
in an ORA assignment. Assessments can be Self, Peer and Staff.

* `list_assessments_from` handler
Lists all assessments received by a user (according to their submissionUUID)
in an ORA assignment. Assessments can be Self, Peer and Staff.

Args:
request (Request): The request object.
usage_id (str): Usage ID of the XBlock for running the handler
handler_name (str): The name of the handler to call
submission_uuid (str): The ORA submission UUID
"""
data = {
"item_id": usage_id,
"submission_uuid": submission_uuid,
}

response = call_xblock_json_handler(request, usage_id, handler_name, data)

if response.status_code != HTTPStatus.OK:
raise XBlockInternalError(context={"handler": handler_name})
mariajgrimaldi marked this conversation as resolved.
Show resolved Hide resolved

try:
return json.loads(response.content)
except json.JSONDecodeError as exc:
raise XBlockInternalError(
context={"handler": handler_name, "details": response.content}
) from exc


def get_submission_info(request, usage_id, submission_uuid):
"""
Get submission content from ORA 'get_submission_info' XBlock.json_handler
Expand Down
68 changes: 67 additions & 1 deletion lms/djangoapps/ora_staff_grader/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,13 @@ class ScoreSerializer(serializers.Serializer):

class SubmissionMetadataSerializer(serializers.Serializer):
"""
Submission metadata for displaying submissions table in ESG
Submission metadata for displaying submissions table in Enhanced Staff Grader (ESG)
"""

submissionUUID = serializers.CharField(source="submissionUuid")
username = serializers.CharField(allow_null=True)
email = serializers.CharField(allow_null=True)
fullname = serializers.CharField(allow_null=True)
teamName = serializers.CharField(allow_null=True)
dateSubmitted = serializers.DateTimeField()
dateGraded = serializers.DateTimeField(allow_null=True)
Expand All @@ -159,6 +161,57 @@ class Meta:
"gradeStatus",
"lockStatus",
"score",
"email",
"fullname",
]
read_only_fields = fields
mariajgrimaldi marked this conversation as resolved.
Show resolved Hide resolved


class AssessmentScoresSerializer(serializers.Serializer):
"""
Serializer for score information associated with a specific assessment.

This serializer is included in the `AssessmentSerializer` as a `ListField`
"""
criterion_name = serializers.CharField()
score_earned = serializers.IntegerField()
score_type = serializers.CharField()

class Meta:
fields = [
"criterion_name",
"score_earned",
"score_type",
]
read_only_fields = fields


class AssessmentSerializer(serializers.Serializer):
"""
Serializer for the each assessment metadata in the response from the assessments
feedback endpoints (from/to) in Enhanced Staff Grader (ESG)

This serializer is included in the `AssessmentFeedbackSerializer` as a `ListField`
"""
assessment_id = serializers.CharField()
scorer_name = serializers.CharField(allow_null=True)
scorer_username = serializers.CharField(allow_null=True)
scorer_email = serializers.CharField(allow_null=True)
assessment_date = serializers.DateTimeField()
assessment_scores = serializers.ListField(child=AssessmentScoresSerializer())
problem_step = serializers.CharField(allow_null=True)
feedback = serializers.CharField(allow_null=True)

class Meta:
fields = [
"assessment_id",
"scorer_name",
"scorer_username",
"scorer_email",
"assessment_date",
"assessment_scores",
"problem_step",
"feedback",
]
read_only_fields = fields

Expand Down Expand Up @@ -190,6 +243,19 @@ def get_isEnabled(self, obj):
return obj['isEnabled'] and not obj['oraMetadata'].teams_enabled


class AssessmentFeedbackSerializer(serializers.Serializer):
"""
Serializer for a list of assessments for the response from the assessments
feedback endpoints (from/to) in Enhanced Staff Grader (ESG)
"""

assessments = serializers.ListField(child=AssessmentSerializer())

class Meta:
fields = ["assessments"]
read_only_fields = fields


class UploadedFileSerializer(serializers.Serializer):
"""Serializer for a file uploaded as a part of a response"""

Expand Down
10 changes: 10 additions & 0 deletions lms/djangoapps/ora_staff_grader/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ class TestSubmissionMetadataSerializer(TestCase):
"submissionUuid": "a",
"username": "foo",
"teamName": "",
'email': "[email protected]",
'fullname': "",
"dateSubmitted": "1969-07-16 13:32:00",
"dateGraded": "None",
"gradedBy": "",
Expand All @@ -251,6 +253,8 @@ class TestSubmissionMetadataSerializer(TestCase):
"b": {
"submissionUuid": "b",
"username": "",
'email': "[email protected]",
'fullname': "Jhon Does",
"teamName": "bar",
"dateSubmitted": "1969-07-20 20:17:40",
"dateGraded": "None",
Expand All @@ -262,6 +266,8 @@ class TestSubmissionMetadataSerializer(TestCase):
"c": {
"submissionUuid": "c",
"username": "baz",
'email': "[email protected]",
'fullname': "Jhon Does",
"teamName": "",
"dateSubmitted": "1969-07-21 21:35:00",
"dateGraded": "1969-07-24 16:44:00",
Expand Down Expand Up @@ -293,6 +299,8 @@ def test_empty_score(self):
submission = {
"submissionUuid": "empty-score",
"username": "WOPR",
'email': "[email protected]",
'fullname': "",
"dateSubmitted": "1983-06-03 00:00:00",
"dateGraded": None,
"gradedBy": None,
Expand All @@ -304,6 +312,8 @@ def test_empty_score(self):
expected_output = {
"submissionUUID": "empty-score",
"username": "WOPR",
'email': "[email protected]",
'fullname': "",
"teamName": None,
"dateSubmitted": "1983-06-03 00:00:00",
"dateGraded": None,
Expand Down
9 changes: 7 additions & 2 deletions lms/djangoapps/ora_staff_grader/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""
from django.urls import include
from django.urls import path

from rest_framework.routers import DefaultRouter

from lms.djangoapps.ora_staff_grader.views import (
InitializeView,
Expand All @@ -13,12 +13,15 @@
SubmissionLockView,
SubmissionStatusFetchView,
UpdateGradeView,
AssessmentFeedbackView,
)


urlpatterns = []
app_name = "ora-staff-grader"

router = DefaultRouter()
router.register("assessments/feedback", AssessmentFeedbackView, basename="assessment-feedback")

urlpatterns += [
path("mock/", include("lms.djangoapps.ora_staff_grader.mock.urls")),
path("initialize", InitializeView.as_view(), name="initialize"),
Expand All @@ -33,3 +36,5 @@
path("submission/grade", UpdateGradeView.as_view(), name="update-grade"),
path("submission", SubmissionFetchView.as_view(), name="fetch-submission"),
]

urlpatterns += router.urls
101 changes: 101 additions & 0 deletions lms/djangoapps/ora_staff_grader/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKey
from openassessment.xblock.config_mixin import WAFFLE_NAMESPACE, ENHANCED_STAFF_GRADER
from rest_framework.decorators import action
from rest_framework.generics import RetrieveAPIView
from rest_framework.permissions import IsAuthenticated
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.viewsets import ViewSet
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError

Expand All @@ -43,11 +46,13 @@
get_assessment_info,
get_submission_info,
get_submissions,
get_assessments,
submit_grade,
)
from lms.djangoapps.ora_staff_grader.serializers import (
FileListSerializer,
InitializeSerializer,
AssessmentFeedbackSerializer,
LockStatusSerializer,
StaffAssessSerializer,
SubmissionFetchSerializer,
Expand Down Expand Up @@ -147,6 +152,102 @@ def get(self, request, ora_location, *args, **kwargs):
return UnknownErrorResponse()


class AssessmentFeedbackView(StaffGraderBaseView, ViewSet):
"""
View for fetching assessment feedback for a submission.

**Methods**

* (GET) `api/ora_staff_grader/assessments/feedback/from`
List all assessments received by a user (according to
their submissionUUID) in an ORA assignment.

* (GET) `api/ora_staff_grader/assessments/feedback/to`
List all assessments given by a user (according to
their submissionUUID) in an ORA assignment.

**Query Params**:

* `oraLocation` (str): ORA location for XBlock handling
* `submissionUUID` (str): The ORA submission UUID

**Response**:

{
assessments (List[dict]): [
{
"assessment_id: (str) Assessment id
"scorer_name: (str) Scorer name
"scorer_username: (str) Scorer username
"scorer_email: (str) Scorer email
"assessment_date: (str) Assessment date
"assessment_scores (List[dict]) [
{
"criterion_name: (str) Criterion name
"score_earned: (int) Score earned
"score_type: (str) Score type
}
]
"problem_step (str) Problem step (Self, Peer, or Staff)
"feedback: (str) Feedback of the assessment
}
]
}

**Errors**:

* `MissingParamResponse` (HTTP 400) for missing params
* `BadOraLocationResponse` (HTTP 400) for bad ORA location
* `XBlockInternalError` (HTTP 500) for an issue with ORA
* `UnknownError` (HTTP 500) for other errors
"""
@action(methods=["get"], detail=False, url_path="from")
@require_params([PARAM_ORA_LOCATION, PARAM_SUBMISSION_ID])
def get_from(self, request: Request, ora_location: str, submission_uuid: str, *args, **kwargs):
return self._get_assessments(request, ora_location, "list_assessments_from", submission_uuid)

@action(methods=["get"], detail=False, url_path="to")
@require_params([PARAM_ORA_LOCATION, PARAM_SUBMISSION_ID])
def get_to(self, request: Request, ora_location: str, submission_uuid: str, *args, **kwargs):
return self._get_assessments(request, ora_location, "list_assessments_to", submission_uuid)

def _get_assessments(
self, request: Request, ora_location: str, handler_name: str, submission_uuid: str
):
"""
Fetches assessment data using the given handler name.

Args:
request (Request): The Django request object.
ora_location (str): The ORA location for XBlock handling.
handler_name (str): The name of the XBlock handler to use.
submission_uuid (str): The ORA submission UUID.

Returns:
A Django response object containing serialized assessment data or an error response.
"""
try:
assessments_data = {
"assessments": get_assessments(
request, ora_location, handler_name, submission_uuid
)
}
response_data = AssessmentFeedbackSerializer(assessments_data).data
return Response(response_data)

except (InvalidKeyError, ItemNotFoundError):
log.error(f"Bad ORA location provided: {ora_location}")
return BadOraLocationResponse()

except XBlockInternalError as ex:
log.error(ex)
return InternalErrorResponse(context=ex.context)

except Exception as ex:
log.exception(ex)
return UnknownErrorResponse()
Comment on lines +246 to +248
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BryanttV I'm hitting this general Exception clause when I try accessing these endpoints as a student. Traceback below; is there a better way to catch xblock handler permission errors?

edx.devstack.lms  | 2024-02-18 12:05:23,986 ERROR 983 [lms.djangoapps.ora_staff_grader.views] [user 6] [ip 172.19.0.1] views.py:243 - Expecting value: line 1 column 1 (char 0)
edx.devstack.lms  | Traceback (most recent call last):
edx.devstack.lms  |   File "/edx/app/edxapp/edx-platform/lms/djangoapps/ora_staff_grader/views.py", line 230, in _get_assessments
edx.devstack.lms  |     assessments_data = {"assessments": getter_func(request, ora_location, submission_uuid)}
edx.devstack.lms  |   File "/edx/app/edxapp/edx-platform/lms/djangoapps/ora_staff_grader/ora_api.py", line 63, in get_assessments_to
edx.devstack.lms  |     return json.loads(response.content)
edx.devstack.lms  |   File "/usr/lib/python3.8/json/__init__.py", line 357, in loads
edx.devstack.lms  |     return _default_decoder.decode(s)
edx.devstack.lms  |   File "/usr/lib/python3.8/json/decoder.py", line 337, in decode
edx.devstack.lms  |     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
edx.devstack.lms  |   File "/usr/lib/python3.8/json/decoder.py", line 355, in raw_decode
edx.devstack.lms  |     raise JSONDecodeError("Expecting value", s, err.value) from None
edx.devstack.lms  | json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
edx.devstack.lms  | Internal Server Error: /api/ora_staff_grader/assessments/feedback/to/
edx.devstack.lms  | 2024-02-18 12:05:24,056 ERROR 983 [django.request] [user None] [ip None] log.py:241 - Internal Server Error: /api/ora_staff_grader/assessments/feedback/to/
edx.devstack.lms  | [18/Feb/2024 12:05:24] "GET /api/ora_staff_grader/assessments/feedback/to/?oraLocation=block-v1%3AedX%2BDemoX%2BDemo_Course%2Btype%40openassessment%2Bblock%40bce6c7ff830a473f9971204964424221&submissionUUID=90fe5b5b-03fa-4371-af26-2ee49939483f HTTP/1.1" 500 19098

Copy link
Contributor Author

@BryanttV BryanttV Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pomegranited, I was doing some research and found the following:

  • Currently, this is how the other ORA Staff Grader endpoints behave.

  • The error is generated from the following line. When there is a permissions error from ORA the following is returned in the response.content:

    b'<div>You do not have permission to access ORA staff grading.</div>\n

    When trying to parse this string to a dict using json.loads() it breaks 💥

I think that this permission error should be raised by the XBlockInternalError, but since the status_code of the response always returns a 200, the error would never be raised, even though it exists. In my opinion, if there is any type of error from the handler, the most appropriate thing to do is to return the corresponding 4xx code.

Copy link
Member

@mariajgrimaldi mariajgrimaldi Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that also happens with the existing initialize endpoint. The views don't check for permissions, but it waits for the ORA handlers to return:

> response.content
b'<div>You do not have permission to access ORA staff grading.</div>\n'.  

So maybe it'd be enough to return this error, but avoiding the unknown error by wrapping json.loads(response.content) in a try-except block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By doing this change:

try:
    return json.loads(response.content)
except json.JSONDecodeError as exc:
    raise XBlockInternalError(
        context={"handler": handler_name, "details": response.content}
    ) from exc

The response looks like this:

{
	"error": "ERR_INTERNAL",
	"handler": "list_assessments_to",
	"details": "<div>You do not have permission to access ORA staff grading.</div>\n"
}

And the log looks like this:

tutor_dev-lms-1  | 2024-02-20 17:49:21,564 ERROR 31 [lms.djangoapps.ora_staff_grader.views] [user 6] [ip 172.18.0.1] views.py:243 - {'handler': 'list_assessments_to', 'details': b'<div>You do not have permission to access ORA staff grading.</div>\n'}
tutor_dev-lms-1  | Internal Server Error: /api/ora_staff_grader/assessments/feedback/to/
tutor_dev-lms-1  | 2024-02-20 17:49:21,668 ERROR 31 [django.request] [user None] [ip None] log.py:241 - Internal Server Error: /api/ora_staff_grader/assessments/feedback/to/
tutor_dev-lms-1  | [20/Feb/2024 17:49:21] "GET /api/ora_staff_grader/assessments/feedback/to/?oraLocation=block-v1%3Aedunext%2BDemo%2BORA%2Btype%40openassessment%2Bblock%4075685668559f451dabd6ebe6375ad080&submissionUUID=29872ddb-4dfd-4f17-b4bb-459ce1167fcc HTTP/1.1" 500 137



class SubmissionFetchView(StaffGraderBaseView):
"""
GET submission contents and assessment info, if any
Expand Down
Loading