-
Notifications
You must be signed in to change notification settings - Fork 0
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: ORA staff grader endpoints upgrade #10
base: FG/ORA_staff_grader_initialize_upgrade
Are you sure you want to change the base?
feat: ORA staff grader endpoints upgrade #10
Conversation
if data['assessment_type'] == "received": | ||
return generate_received_assessment_data(subission_uuid) | ||
else: | ||
return generate_given_assessment_data(item_id, subission_uuid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this given
and received
as the assessment type present elsewhere? can we elaborate on the meaning of each?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the default should be list all assessments for the submission uuid, so received.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is a single handler better than separating concerns? list_assessments_received
or list_assessments_given
are more explicit than list_assessments...
, but I don't have other strong arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the MFE, it's just a table with a filter button; the data output has the same format. At the backend level, it's just a design decision that, compared to having two handlers, neither adds nor subtracts anything; it's pretty much the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not the same. When using a single handler with a variable assessment_type
, which defaults to given, you are making a decision there, you're saying the defaults assessments returned are given by students on the submission. When using two handlers, the client can actively decide which one to call without guessing the system's default. Don't you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! that's true... but it does not exists a request without assessment_type (now called assessment_filter
), it will always be given
or received
...
In fact, it is a required param in the AssessmentFeedbackView
of edx-platform and declared in lms/djangoapps/ora_staff_grader/constants.py so, list_assessments() would be never called without assessment_filter...
jeje, but now tha you are saying this... I think that in form, this will look even better 😃
filter_value = data['assessment_filter']
if filter_value == "received":
return generate_received_assessment_data(subission_uuid)
elif filter_value == "given":
return generate_given_assessment_data(item_id, subission_uuid)
else:
raise ValueError("Invalid assessment_filter value")
for optimized user data retrieval.
ORA staff grader endpoints upgrade
For this PR we have the following goals:
/api/ora_staff_grader/assessments/feedback
endpoint in order to list the assessments grades based on the type (received or given) for a specific submission./api/ora_staff_grader/initialize
endpoint in order to add thefullname
andemail
fields.Descriptions
/api/ora_staff_grader/assessments/feedback
This endpoint fetches assessment details for a given submission UUID, assessment_fillter [
given
,received
] and ORA_location, including scorer information and scoring details in order to fill the following Staff Grade MFE table:Click here! to see `/api/ora_staff_grader/assessments/feedback` output
/api/ora_staff_grader/initialize
This is a pre-existing endpoint to which two extra fields are added [
fullname
,email
].Modifications were made in the following directories:
Click here! to see `/api/ora_staff_grader/initialize` output
Testing
In order to test this, you can use the API platform Postman:
given
,received
]This upgrade will consist of two PRs: one for ORA and the other for edx-platform. This is because the main serializer of the service resides in the platform. To test it, you'll need to fetch and set up the version of the platform with the serializer modification.